Repository: qpid-proton Updated Branches: refs/heads/0.13.x ad532c392 -> 774921021
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. Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/77492102 Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/77492102 Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/77492102 Branch: refs/heads/0.13.x Commit: 7749210210e339b286b22067a1c1104e7c6fa370 Parents: ad532c3 Author: Dominic Evans <[email protected]> Authored: Fri Mar 18 11:20:23 2016 +0000 Committer: Justin Ross <[email protected]> Committed: Fri Jul 1 08:33:02 2016 -0700 ---------------------------------------------------------------------- .../org/apache/qpid/proton/codec/ArrayType.java | 17 ++++++++++++++--- .../org/apache/qpid/proton/codec/BinaryType.java | 9 +++++++-- .../qpid/proton/codec/ByteBufferDecoder.java | 2 ++ .../org/apache/qpid/proton/codec/DecoderImpl.java | 4 ++++ .../org/apache/qpid/proton/codec/ListType.java | 5 +++++ .../java/org/apache/qpid/proton/codec/MapType.java | 4 ++++ 6 files changed, 36 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/77492102/proton-j/src/main/java/org/apache/qpid/proton/codec/ArrayType.java ---------------------------------------------------------------------- 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 @@ public class ArrayType implements PrimitiveType<Object[]> 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 @@ public class ArrayType implements PrimitiveType<Object[]> 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 @@ public class ArrayType implements PrimitiveType<Object[]> } else { - return decodeNonPrimitive(constructor, count); + return decodeNonPrimitive(decoder, constructor, count); } } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/77492102/proton-j/src/main/java/org/apache/qpid/proton/codec/BinaryType.java ---------------------------------------------------------------------- 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 class BinaryType extends AbstractPrimitiveType<Binary> 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); } } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/77492102/proton-j/src/main/java/org/apache/qpid/proton/codec/ByteBufferDecoder.java ---------------------------------------------------------------------- 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 @@ import java.nio.ByteBuffer; public interface ByteBufferDecoder extends Decoder { public void setByteBuffer(ByteBuffer buffer); + + public int getByteBufferRemaining(); } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/77492102/proton-j/src/main/java/org/apache/qpid/proton/codec/DecoderImpl.java ---------------------------------------------------------------------- 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 class DecoderImpl implements ByteBufferDecoder } } + + public int getByteBufferRemaining() { + return _buffer.remaining(); + } } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/77492102/proton-j/src/main/java/org/apache/qpid/proton/codec/ListType.java ---------------------------------------------------------------------- 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 @@ public class ListType extends AbstractPrimitiveType<List> 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++) { http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/77492102/proton-j/src/main/java/org/apache/qpid/proton/codec/MapType.java ---------------------------------------------------------------------- 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 @@ public class MapType extends AbstractPrimitiveType<Map> 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++) { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
