You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by dn...@apache.org on 2016/06/30 21:48:51 UTC

qpid-proton git commit: PROTON-1249: Safeguard type initialisations.

Repository: qpid-proton
Updated Branches:
  refs/heads/master 63bb49c94 -> 0c9282886


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/0c928288
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/0c928288
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/0c928288

Branch: refs/heads/master
Commit: 0c92828864118517f8d8a23a2801da82852cf36c
Parents: 63bb49c
Author: Dominic Evans <do...@uk.ibm.com>
Authored: Fri Mar 18 11:20:23 2016 +0000
Committer: Dominic Evans <do...@uk.ibm.com>
Committed: Thu Jun 30 13:03:13 2016 +0100

----------------------------------------------------------------------
 .../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/0c928288/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/0c928288/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/0c928288/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/0c928288/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/0c928288/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/0c928288/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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org