fix #185 (possible stackoverflow decoding crafted nested tagged arrays)

Author: cowtowncoder

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

@@ -877,36 +877,195 @@ protected JsonToken _handleTaggedArray(int tag, int len) throws IOException
             _reportError("Unexpected array size ("+len+") for tagged 'bigfloat' value; should have exactly 2 number elements");
         }
         // and then use recursion to get values
-        JsonToken t = nextToken();
         // First: exponent, which MUST be a simple integer value
-        if (t != JsonToken.VALUE_NUMBER_INT) {
-            _reportError("Unexpected token ("+t+") as the first part of 'bigfloat' value: should get VALUE_NUMBER_INT");
+        if (!_checkNextIsIntInArray("bigfloat")) {
+            _reportError("Unexpected token ("+currentToken()+") as the first part of 'bigfloat' value: should get VALUE_NUMBER_INT");
         }
         // 27-Nov-2019, tatu: As per [dataformats-binary#139] need to change sign here
         int exp = -getIntValue();
 
-        t = nextToken();
         // Should get an integer value; int/long/BigInteger
-        if (t != JsonToken.VALUE_NUMBER_INT) {
-            _reportError("Unexpected token ("+t+") as the second part of 'bigfloat' value: should get VALUE_NUMBER_INT");
+        if (!_checkNextIsIntInArray("bigfloat")) {
+            _reportError("Unexpected token ("+currentToken()+") as the second part of 'bigfloat' value: should get VALUE_NUMBER_INT");
         }
-        
-        BigDecimal dec;
 
+        // important: check number type here
+        BigDecimal dec;
         NumberType numberType = getNumberType();
         if (numberType == NumberType.BIG_INTEGER) {
             dec = new BigDecimal(getBigIntegerValue(), exp);
         } else  {
             dec = BigDecimal.valueOf(getLongValue(), exp);
         }
-        t = nextToken();
-        if (t != JsonToken.END_ARRAY) {
-            _reportError("Unexpected token ("+t+") after 2 elements of 'bigfloat' value");
+
+        // but verify closing END_ARRAY here, as this will now override current token
+        if (!_checkNextIsEndArray()) {
+            _reportError("Unexpected token ("+currentToken()+") after 2 elements of 'bigfloat' value");
         }
+
+        // which needs to be reset here
         _numberBigDecimal = dec;
         _numTypesValid = NR_BIGDECIMAL;
         return (_currToken = JsonToken.VALUE_NUMBER_FLOAT);
     }
+
+    /**
+     * Heavily simplified method that does a subset of what {@code nextTokendoes to basically
+     * only (1) determine that we are getting {@code JsonToken.VALUE_NUMBER_INT} (if not,
+     * return with no processing) and (2) if so, prepare state so that number accessor
+     * method will work).
+     *<p>
+     * Note that in particular this method DOES NOT reset state that {@code nextToken()} would do,
+     * but will change current token type to allow access.
+     */
+    protected final boolean _checkNextIsIntInArray(final String typeDesc) throws IOException
+    {
+        // We know we are in array, with length prefix so:
+        if (!_parsingContext.expectMoreValues()) {
+            _tagValue = -1;
+            _parsingContext = _parsingContext.getParent();
+            _currToken = JsonToken.END_ARRAY;
+            return false;
+        }
+
+        if (_inputPtr >= _inputEnd) {
+            if (!loadMore()) {
+                _handleCBOREOF();
+                return false;
+            }
+        }
+        int ch = _inputBuffer[_inputPtr++];
+        int type = (ch >> 5) & 0x7;
+
+        // 01-Nov-2019, tatu: Although we don't use tag for anything, might as well decode it
+        //    for funsies and just ignore.
+        int tagValue = -1;
+        if (type == 6) {
+            tagValue = _decodeTag(ch & 0x1F);
+            if (_inputPtr >= _inputEnd) {
+                if (!loadMore()) {
+                    _handleCBOREOF();
+                    return false;
+                }
+            }
+            ch = _inputBuffer[_inputPtr++];
+            type = (ch >> 5) & 0x7;
+        }
+        
+        final int lowBits = ch & 0x1F;
+        switch (type) {
+        case 0: // positive int
+            _numTypesValid = NR_INT;
+            if (lowBits <= 23) {
+                _numberInt = lowBits;
+            } else {
+                switch (lowBits - 24) {
+                case 0:
+                    _numberInt = _decode8Bits();
+                    break;
+                case 1:
+                    _numberInt = _decode16Bits();
+                    break;
+                case 2:
+                    {
+                        int v = _decode32Bits();
+                        if (v >= 0) {
+                            _numberInt = v;
+                        } else {
+                            long l = (long) v;
+                            _numberLong = l & 0xFFFFFFFFL;
+                            _numTypesValid = NR_LONG;
+                        }
+                    }
+                    break;
+                case 3:
+                    {
+                        long l = _decode64Bits();
+                        if (l >= 0L) {
+                            _numberLong = l;
+                            _numTypesValid = NR_LONG;
+                        } else {
+                            _numberBigInt = _bigPositive(l);
+                            _numTypesValid = NR_BIGINT;
+                        }
+                    }
+                    break;
+                default:
+                    _invalidToken(ch);
+                }
+            }
+            _currToken = JsonToken.VALUE_NUMBER_INT;
+            return true;
+        case 1: // negative int
+            _numTypesValid = NR_INT;
+            if (lowBits <= 23) {
+                _numberInt = -lowBits - 1;
+            } else {
+                switch (lowBits - 24) {
+                case 0:
+                    _numberInt = -_decode8Bits() - 1;
+                    break;
+                case 1:
+                    _numberInt = -_decode16Bits() - 1;
+                    break;
+                case 2:
+                    // 15-Oct-2016, as per [dataformats-binary#30], we got an edge case here
+                    {
+                        int v = _decode32Bits();
+                        if (v < 0) {
+                            long unsignedBase = (long) v & 0xFFFFFFFFL;
+                            _numberLong = -unsignedBase - 1L;
+                            _numTypesValid = NR_LONG;
+                        } else {
+                            _numberInt = -v - 1;
+                        }
+                    }
+                    break;
+                case 3:
+                    // 15-Oct-2016, as per [dataformats-binary#30], we got an edge case here
+                    {
+                        long l = _decode64Bits();
+                        if (l >= 0L) {
+                            _numberLong = -l - 1L;
+                            _numTypesValid = NR_LONG;
+                        } else {
+                            _numberBigInt = _bigNegative(l);
+                            _numTypesValid = NR_BIGINT;
+                        }
+                    }
+                    break;
+                default:
+                    _invalidToken(ch);
+                }
+            }
+            _currToken = JsonToken.VALUE_NUMBER_INT;
+            return true;
+
+        case 2: // byte[]
+            // ... but we only really care about very specific case of `BigInteger`
+            if (tagValue < 0) {
+                break;
+            }
+            _typeByte = ch;
+            _tokenIncomplete = true;
+            _currToken = _handleTaggedBinary(tagValue);
+            return (_currToken == JsonToken.VALUE_NUMBER_INT);
+
+        case 6: // another tag; not allowed
+            _reportError("Multiple tags not allowed per value (first tag: "+tagValue+")");
+        }
+
+        // Important! Need to push back the last byte read (but not consumed)
+        --_inputPtr;
+        // and now it is safe to decode next token, too
+        nextToken();
+        return false;
+    }
+
+    protected final boolean _checkNextIsEndArray() throws IOException
+    {
+        return nextToken() == JsonToken.END_ARRAY;
+    }
 
     // base impl is fine:
     //public String getCurrentName() throws IOException
@@ -1467,7 +1626,7 @@ public byte[] getBinaryValue(Base64Variant b64variant) throws IOException
         }
         if (_currToken != JsonToken.VALUE_EMBEDDED_OBJECT ) {
             // TODO, maybe: support base64 for text?
-            _reportError("Current token ("+getCurrentToken()+") not VALUE_EMBEDDED_OBJECT, can not access as binary");
+            _reportError("Current token ("+currentToken()+") not VALUE_EMBEDDED_OBJECT, can not access as binary");
         }
         return _binaryValue;
     }
@@ -1489,7 +1648,7 @@ public int readBinaryValue(Base64Variant b64variant, OutputStream out) throws IO
     {
         if (_currToken != JsonToken.VALUE_EMBEDDED_OBJECT ) {
             // Todo, maybe: support base64 for text?
-            _reportError("Current token ("+getCurrentToken()+") not VALUE_EMBEDDED_OBJECT, can not access as binary");
+            _reportError("Current token ("+currentToken()+") not VALUE_EMBEDDED_OBJECT, can not access as binary");
         }
         if (!_tokenIncomplete) { // someone already decoded or read
             if (_binaryValue == null) { // if this method called twice in a row
@@ -1724,7 +1883,7 @@ protected void _checkNumericValue(int expType) throws IOException
         if (_currToken == JsonToken.VALUE_NUMBER_INT || _currToken == JsonToken.VALUE_NUMBER_FLOAT) {
             return;
         }
-        _reportError("Current token ("+getCurrentToken()+") not numeric, can not use numeric value accessors");
+        _reportError("Current token ("+currentToken()+") not numeric, can not use numeric value accessors");
     }
 
     protected void convertNumberToInt() throws IOException
@@ -2770,11 +2929,11 @@ private final int _decodeExplicitLength(int lowBits) throws IOException
         case 3:
             long l = _decode64Bits();
             if (l < 0 || l > MAX_INT_L) {
-                throw _constructError("Illegal length for "+getCurrentToken()+": "+l);
+                throw _constructError("Illegal length for "+currentToken()+": "+l);
             }
             return (int) l;
         }
-        throw _constructError("Invalid length for "+getCurrentToken()+": 0x"+Integer.toHexString(lowBits));
+        throw _constructError("Invalid length for "+currentToken()+": 0x"+Integer.toHexString(lowBits));
     }
 
     private int _decodeChunkLength(int expType) throws IOException

cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/BigNumbersTest.java

@@ -12,7 +12,7 @@
 // tests for [cbor#17]
 public class BigNumbersTest extends CBORTestBase
 {
-    public void testBigDecimal() throws Exception
+    public void testBigDecimalShort() throws Exception
     {
         _testBigDecimal(BigDecimal.ONE);
         _testBigDecimal(BigDecimal.ZERO);
@@ -31,9 +31,12 @@ public void testBigDecimal() throws Exception
         BigDecimal bd = new BigDecimal("12345.667899024");
         _testBigDecimal(bd);
         _testBigDecimal(bd.negate());
+    }
 
+    public void testBigDecimalLonger() throws Exception
+    {
         // ensure mantissa is beyond long; more than 22 digits or so
-        bd = new BigDecimal("1234567890.12345678901234567890");
+        BigDecimal bd = new BigDecimal("1234567890.12345678901234567890");
         _testBigDecimal(bd);
         _testBigDecimal(bd.negate());
     }

cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/ParserNumbersTest.java

@@ -343,7 +343,9 @@ public void testBigDecimalType() throws IOException {
             assertEquals(NR.intValue(), parser.getIntValue());
             assertNull(parser.nextToken());
         }
+    }
 
+    public void testBigDecimalType2() throws IOException {
         // Almost good. But [dataformats#139] to consider too, see
         // [https://tools.ietf.org/html/rfc7049#section-2.4.2]
         final byte[] spec = new byte[] {

…rmat/cbor/failing/TagParsing185Test.java …format/cbor/parse/TagParsing185Test.javacbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/failing/TagParsing185Test.java renamed to cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/TagParsing185Test.java cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/failing/TagParsing185Test.java renamed to cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/TagParsing185Test.java

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.dataformat.cbor.failing;
+package com.fasterxml.jackson.dataformat.cbor.parse;
 
 import com.fasterxml.jackson.core.*;
 
@@ -24,7 +24,14 @@ private void _testRecursiveTags(int levels) throws Exception
          }
 
          JsonParser p = CBOR_F.createParser(data);
+         JsonToken t;
 
-         assertToken(JsonToken.START_ARRAY, p.nextToken());
+         try {
+             t = p.nextToken();
+             fail("Should not pass, got token: "+t);
+         } catch (JsonParseException e) {
+             verifyException(e, "Unexpected token");
+             verifyException(e, "first part of 'bigfloat' value");
+         }
     }
 }

release-notes/CREDITS-2.x

@@ -108,3 +108,7 @@ Marcos Passos (marcospassos@github)
 John (iziamos@github)
 * Reported, suggested fix for #178: Fix issue wit input offsets when parsing CBOR from `InputStream`
  (2.10.0)
+
+Paul Adolph (padolph@github)
+* Reported #185: Internal parsing of tagged arrays can lead to stack overflow
+ (2.10.1)

release-notes/VERSION-2.x

@@ -8,6 +8,11 @@ Project: jackson-datatypes-binaryModules:
 === Releases ===
 ------------------------------------------------------------------------
 
+2.10.1 (not yet released)
+
+#185: Internal parsing of tagged arrays can lead to stack overflow
+ (reported by Paul A)
+
 2.10.0 (26-Sep-2019)
 
 #139: (cbor) Incorrect decimal fraction representation