PROTON-1249: Safeguard type initialisations.
In #readValue() for ArrayType, BinaryType, ListType and MapType
decoding, if the 'count' specified is very large then it is likely to
trigger an OutOfMemoryException. As these can come from an external data
source, during the SASL init for example, there is a potential for a
denial of service. The fix is to throw an IllegalArgumentException if
the count value is larger than the amount of data available in the
received bytes.
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/ArrayType.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/ArrayType.java
index 11a1dc9..45b8dd5 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/ArrayType.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/ArrayType.java
@@ -970,12 +970,18 @@
private static Object[] decodeArray(final DecoderImpl decoder, final int count)
{
TypeConstructor constructor = decoder.readConstructor();
- return decodeNonPrimitive(constructor, count);
+ return decodeNonPrimitive(decoder, constructor, count);
}
- private static Object[] decodeNonPrimitive(final TypeConstructor constructor,
+ private static Object[] decodeNonPrimitive(final DecoderImpl decoder,
+ final TypeConstructor constructor,
final int count)
{
+ if (count > decoder.getByteBufferRemaining()) {
+ throw new IllegalArgumentException("Array element count "+count+" is specified to be greater than the amount of data available ("+
+ decoder.getByteBufferRemaining()+")");
+ }
+
if(constructor instanceof ArrayEncoding)
{
ArrayEncoding arrayEncoding = (ArrayEncoding) constructor;
@@ -1006,6 +1012,11 @@
TypeConstructor constructor = decoder.readConstructor();
if(constructor.encodesJavaPrimitive())
{
+ if (count > decoder.getByteBufferRemaining()) {
+ throw new IllegalArgumentException("Array element count "+count+" is specified to be greater than the amount of data available ("+
+ decoder.getByteBufferRemaining()+")");
+ }
+
if(constructor instanceof BooleanType.BooleanEncoding)
{
return decodeBooleanArray((BooleanType.BooleanEncoding) constructor, count);
@@ -1042,7 +1053,7 @@
}
else
{
- return decodeNonPrimitive(constructor, count);
+ return decodeNonPrimitive(decoder, constructor, count);
}
}
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/BinaryType.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/BinaryType.java
index 6b463e4..88c204f 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/BinaryType.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/BinaryType.java
@@ -105,9 +105,14 @@
public Binary readValue()
{
- int size = getDecoder().readRawInt();
+ final DecoderImpl decoder = getDecoder();
+ int size = decoder.readRawInt();
+ if (size > decoder.getByteBufferRemaining()) {
+ throw new IllegalArgumentException("Binary data size "+size+" is specified to be greater than the amount of data available ("+
+ decoder.getByteBufferRemaining()+")");
+ }
byte[] data = new byte[size];
- getDecoder().readRaw(data, 0, size);
+ decoder.readRaw(data, 0, size);
return new Binary(data);
}
}
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/ByteBufferDecoder.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/ByteBufferDecoder.java
index 4a10d76..39718b7 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/ByteBufferDecoder.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/ByteBufferDecoder.java
@@ -25,4 +25,6 @@
public interface ByteBufferDecoder extends Decoder
{
public void setByteBuffer(ByteBuffer buffer);
+
+ public int getByteBufferRemaining();
}
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java
index 450ad0e..dd68f6a 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java
@@ -992,4 +992,8 @@
}
}
+
+ public int getByteBufferRemaining() {
+ return _buffer.remaining();
+ }
}
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/ListType.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/ListType.java
index 0c726b2..185373f 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/ListType.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/ListType.java
@@ -155,6 +155,11 @@
int size = decoder.readRawInt();
// todo - limit the decoder with size
int count = decoder.readRawInt();
+ // Ensure we do not allocate an array of size greater then the available data, otherwise there is a risk for an OOM error
+ if (count > decoder.getByteBufferRemaining()) {
+ throw new IllegalArgumentException("List element count "+count+" is specified to be greater than the amount of data available ("+
+ decoder.getByteBufferRemaining()+")");
+ }
List list = new ArrayList(count);
for(int i = 0; i < count; i++)
{
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/codec/MapType.java b/proton-j/src/main/java/org/apache/qpid/proton/codec/MapType.java
index 53aac4f..5c8a7c7 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/codec/MapType.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/codec/MapType.java
@@ -150,6 +150,10 @@
int size = decoder.readRawInt();
// todo - limit the decoder with size
int count = decoder.readRawInt();
+ if (count > decoder.getByteBufferRemaining()) {
+ throw new IllegalArgumentException("Map element count "+count+" is specified to be greater than the amount of data available ("+
+ decoder.getByteBufferRemaining()+")");
+ }
Map map = new LinkedHashMap(count);
for(int i = 0; i < count; i++)
{
