Fix eager allocation aspect of #186

Author: cowtowncoder

cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORGenerator.java

@@ -198,7 +198,7 @@ public int getMask() {
 
     /**
      * Number of elements remaining in the current complex structure (if any),
-     * when writing defined-length Arrays, Objects; marker {@link #INDEFINITE_LENGTH}
+     * when writing defined-length Arrays, Objects; marker {code INDEFINITE_LENGTH}
      * otherwise.
      */
     protected int _currentRemainingElements = INDEFINITE_LENGTH;

cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java

@@ -63,6 +63,10 @@ private Feature(boolean defaultState) {
     private final static double MATH_POW_2_10 = Math.pow(2, 10);
     private final static double MATH_POW_2_NEG14 = Math.pow(2, -14);
 
+    // 2.11.4: [dataformats-binary#186] Avoid OOME/DoS for bigger binary;
+    //  read only up to 250k
+    protected final static int LONGEST_NON_CHUNKED_BINARY = 250_000;
+
     /*
     /**********************************************************
     /* Configuration
@@ -1706,13 +1710,15 @@ public int readBinaryValue(Base64Variant b64variant, OutputStream out) throws IO
         }
     }
 
-    private int _readAndWriteBytes(OutputStream out, int total) throws IOException
+    private int _readAndWriteBytes(OutputStream out, final int total) throws IOException
     {
         int left = total;
         while (left > 0) {
             int avail = _inputEnd - _inputPtr;
             if (_inputPtr >= _inputEnd) {
-                loadMoreGuaranteed();
+                if (!loadMore()) {
+                    _reportIncompleteBinaryRead(total, total-left);
+                }
                 avail = _inputEnd - _inputPtr;
             }
             int count = Math.min(avail, left);
@@ -2425,33 +2431,55 @@ private final int _nextChunkedByte2() throws IOException
         // either way, got it now
         return _inputBuffer[_inputPtr++];
     }
-    
+
+    /**
+     * Helper called to complete reading of binary data ("byte string") in
+     * case contents are needed.
+     */
     @SuppressWarnings("resource")
     protected byte[] _finishBytes(int len) throws IOException
     {
+        // Chunked?
         // First, simple: non-chunked
-        if (len >= 0) {
+        if (len <= 0) {
             if (len == 0) {
                 return NO_BYTES;
             }
-            byte[] b = new byte[len];
-            if (_inputPtr >= _inputEnd) {
-                loadMoreGuaranteed();
+            return _finishChunkedBytes();
+        }
+        // Non-chunked, contiguous
+        if (len > LONGEST_NON_CHUNKED_BINARY) {
+            // [dataformats-binary#186]: avoid immediate allocation for longest
+            return _finishLongContiguousBytes(len);
+        }
+
+        final byte[] b = new byte[len];
+        final int expLen = len;
+        if (_inputPtr >= _inputEnd) {
+            if (!loadMore()) {
+                _reportIncompleteBinaryRead(expLen, 0);
             }
-            int ptr = 0;
-            while (true) {
-                int toAdd = Math.min(len, _inputEnd - _inputPtr);
-                System.arraycopy(_inputBuffer, _inputPtr, b, ptr, toAdd);
-                _inputPtr += toAdd;
-                ptr += toAdd;
-                len -= toAdd;
-                if (len <= 0) {
-                    return b;
-                }
-                loadMoreGuaranteed();
+        }
+
+        int ptr = 0;
+        while (true) {
+            int toAdd = Math.min(len, _inputEnd - _inputPtr);
+            System.arraycopy(_inputBuffer, _inputPtr, b, ptr, toAdd);
+            _inputPtr += toAdd;
+            ptr += toAdd;
+            len -= toAdd;
+            if (len <= 0) {
+                return b;
+            }
+            if (!loadMore()) {
+                _reportIncompleteBinaryRead(expLen, ptr);
             }
         }
+    }
 
+    // @since 2.12
+    protected byte[] _finishChunkedBytes() throws IOException
+    {
         // or, if not, chunked...
         ByteArrayBuilder bb = _getByteArrayBuilder();
         while (true) {
@@ -2468,14 +2496,17 @@ protected byte[] _finishBytes(int len) throws IOException
                 throw _constructError("Mismatched chunk in chunked content: expected "+CBORConstants.MAJOR_TYPE_BYTES
                         +" but encountered "+type);
             }
-            len = _decodeExplicitLength(ch & 0x1F);
+            int len = _decodeExplicitLength(ch & 0x1F);
             if (len < 0) {
                 throw _constructError("Illegal chunked-length indicator within chunked-length value (type "+CBORConstants.MAJOR_TYPE_BYTES+")");
             }
+            final int chunkLen = len;
             while (len > 0) {
                 int avail = _inputEnd - _inputPtr;
                 if (_inputPtr >= _inputEnd) {
-                    loadMoreGuaranteed();
+                    if (!loadMore()) {
+                        _reportIncompleteBinaryRead(chunkLen, chunkLen-len);
+                    }
                     avail = _inputEnd - _inputPtr;
                 }
                 int count = Math.min(avail, len);
@@ -2486,7 +2517,33 @@ protected byte[] _finishBytes(int len) throws IOException
         }
         return bb.toByteArray();
     }
-    
+
+    // @since 2.12
+    protected byte[] _finishLongContiguousBytes(final int expLen) throws IOException
+    {
+        int left = expLen;
+
+        // 04-Dec-2020, tatu: Let's NOT use recycled instance since we have much
+        //   longer content and there is likely less benefit of trying to recycle
+        //   segments
+        try (final ByteArrayBuilder bb = new ByteArrayBuilder(LONGEST_NON_CHUNKED_BINARY >> 1)) {
+            while (left > 0) {
+                int avail = _inputEnd - _inputPtr;
+                if (avail <= 0) {
+                    if (!loadMore()) {
+                        _reportIncompleteBinaryRead(expLen, expLen-left);
+                    }
+                    avail = _inputEnd - _inputPtr;
+                }
+                int count = Math.min(avail, left);
+                bb.write(_inputBuffer, _inputPtr, count);
+                _inputPtr += count;
+                left -= count;        
+            }
+            return bb.toByteArray();
+        }
+    }
+
     protected final JsonToken _decodeFieldName() throws IOException
     {     
         if (_inputPtr >= _inputEnd) {
@@ -2635,9 +2692,8 @@ protected final void _decodeNonStringName(int ch) throws IOException
         } else if (type == CBORConstants.MAJOR_TYPE_INT_NEG) {
             name = _numberToName(ch, true);
         } else if (type == CBORConstants.MAJOR_TYPE_BYTES) {
-            /* 08-Sep-2014, tatu: As per [Issue#5], there are codecs
-             *   (f.ex. Perl module "CBOR::XS") that use Binary data...
-             */
+            // 08-Sep-2014, tatu: As per [Issue#5], there are codecs
+            //   (f.ex. Perl module "CBOR::XS") that use Binary data...
             final int blen = _decodeExplicitLength(ch & 0x1F);
             byte[] b = _finishBytes(blen);
             // TODO: Optimize, if this becomes commonly used & bottleneck; we have
@@ -3204,7 +3260,7 @@ private final int _decodeChunkedUTF8_4(int c) throws IOException
     /**********************************************************
      */
 
-    protected final boolean loadMore() throws IOException
+    protected boolean loadMore() throws IOException
     {
         if (_inputStream != null) {
             _currInputProcessed += _inputEnd;
@@ -3225,7 +3281,7 @@ protected final boolean loadMore() throws IOException
         return false;
     }
 
-    protected final void loadMoreGuaranteed() throws IOException {
+    protected void loadMoreGuaranteed() throws IOException {
         if (!loadMore()) { _reportInvalidEOF(); }
     }
 
@@ -3351,6 +3407,13 @@ protected void _reportInvalidOther(int mask, int ptr) throws JsonParseException
         _reportInvalidOther(mask);
     }
 
+    // @since 2.12
+    protected void _reportIncompleteBinaryRead(int expLen, int actLen) throws IOException
+    {
+        _reportInvalidEOF(String.format(" for Binary value: expected %d bytes, only found %d",
+                expLen, actLen), _currToken);
+    }
+
     /*
     /**********************************************************
     /* Internal methods, other

cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/BrokenLongBinary186Test.java

@@ -0,0 +1,93 @@
+package com.fasterxml.jackson.dataformat.cbor;
+
+import java.io.ByteArrayOutputStream;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.core.JsonToken;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+// Mostly for [dataformats-binary#186]: corrupt encoding indicating humongous payload
+public class BrokenLongBinary186Test extends CBORTestBase
+{
+    private final ObjectMapper MAPPER = cborMapper();
+
+    /*
+    /**********************************************************************
+    /* First regular, read-it-all access, from non-chunked
+    /**********************************************************************
+     */
+
+    // [dataformats-binary#186]
+    public void testCorruptVeryLongBinary() throws Exception {
+        // Let's do about 2 GB to likely trigger failure
+        _testCorruptLong(1_999_999_999, 95000);
+    }
+
+    // [dataformats-binary#186]
+    public void testCorruptQuiteLongBinary() throws Exception {
+        // Value below limit for chunked handling
+        _testCorruptLong(CBORParser.LONGEST_NON_CHUNKED_BINARY >> 1, 37);
+    }
+
+    private void _testCorruptLong(int allegedLength, int actualIncluded) throws Exception
+    {
+        JsonParser p = MAPPER.createParser(_createBrokenDoc(allegedLength, actualIncluded));
+        assertEquals(JsonToken.VALUE_EMBEDDED_OBJECT, p.nextToken());
+        try {
+            p.getBinaryValue();
+            fail("Should fail");
+        } catch (JsonProcessingException e) {
+            verifyException(e, "Unexpected end-of-input for Binary value");
+            verifyException(e, "expected "+allegedLength+" bytes, only found "+actualIncluded);
+        }
+    }
+
+    /*
+    /**********************************************************************
+    /* And then "streaming" access
+    /**********************************************************************
+     */
+
+    // [dataformats-binary#186]
+    public void testQuiteLongStreaming() throws Exception
+    {
+        // Can try bit shorter here, like 500 megs
+        final int allegedLength = 500_000_000;
+        
+        JsonParser p = MAPPER.createParser(_createBrokenDoc(allegedLength, 72000));
+        assertEquals(JsonToken.VALUE_EMBEDDED_OBJECT, p.nextToken());
+        try {
+            ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+            p.readBinaryValue(bytes);
+            fail("Should fail");
+        } catch (JsonProcessingException e) {
+            verifyException(e, "Unexpected end-of-input for Binary value");
+            verifyException(e, "expected "+allegedLength+" bytes, only found 72000");
+        }
+    }
+
+    private byte[] _createBrokenDoc(int allegedLength, int actualIncluded) throws Exception
+    {
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+
+        if (allegedLength > 0xFFFF) {
+            bytes.write((byte) (CBORConstants.PREFIX_TYPE_BYTES | CBORConstants.SUFFIX_UINT32_ELEMENTS));
+            bytes.write((byte) (allegedLength >> 24));
+            bytes.write((byte) (allegedLength >> 16));
+            bytes.write((byte) (allegedLength >> 8));
+            bytes.write((byte) allegedLength);
+        } else { // assume shorter
+            bytes.write((byte) (CBORConstants.PREFIX_TYPE_BYTES | CBORConstants.SUFFIX_UINT16_ELEMENTS));
+            bytes.write((byte) (allegedLength >> 8));
+            bytes.write((byte) allegedLength);
+        }
+        // but only include couple of bytes
+        for (int i = 0; i < actualIncluded; ++i) {
+            bytes.write((byte) i);
+        }
+        return bytes.toByteArray();
+    }
+
+}

cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/failing/BrokenLongBinary186Test.java

@@ -116,8 +116,11 @@ John (iziamos@github)
  (2.10.0)
 
 Paul Adolph (padolph@github)
-* Reported #185: Internal parsing of tagged arrays can lead to stack overflow
+* Reported #185: (cbor) Internal parsing of tagged arrays can lead to stack overflow
  (2.10.1)
+* Reported #186: (cbor) Eager allocation of byte buffer can cause `java.lang.OutOfMemoryError`
+   exception
+ (2.11.4)
 
 Yanming Zhou (quaff@github)
 * Reported #188: Unexpected `MismatchedInputException` for `byte[]` value bound to `String`

release-notes/CREDITS-2.x

@@ -8,9 +8,15 @@ Project: jackson-datatypes-binaryModules:
 === Releases ===
 ------------------------------------------------------------------------
 
+2.11.4 (not yet released)
+
+#186: (cbor) Eager allocation of byte buffer can cause `java.lang.OutOfMemoryError`
+   exception
+ (reported by Paul A)
+
 2.11.3 (02-Oct-2020)
 
-#219: Cache record names to avoid hitting class loader
+#219: (avro) Cache record names to avoid hitting class loader
  (contributed by Marcos P)
 
 2.11.2 (02-Aug-2020)