Merge pull request #125 from JacekLach/jl/negative-number-overflow

Avoid integer overflow when parsing cbor negative numbers
FasterXML · Dec 3, 2017 · 3f29c6c · 3f29c6c
2 parents 13866c9 + ef2aee0
commit 3f29c6c
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 10 deletions.
diff --git a/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java b/cbor/src/main/java/com/fasterxml/jackson/dataformat/cbor/CBORParser.java
@@ -685,7 +685,8 @@ public JsonToken nextToken() throws IOException
                     {
                         int v = _decode32Bits();
                         if (v < 0) {
-                            _numberLong = ((long) v) + -1L;
+                            long unsignedBase = (long) v & 0xFFFFFFFFL;
+                            _numberLong = -unsignedBase - 1L;
                             _numTypesValid = NR_LONG;
                         } else {
                             _numberInt = -v - 1;
@@ -700,7 +701,8 @@ public JsonToken nextToken() throws IOException
                             _numberLong = -l - 1L;
                             _numTypesValid = NR_LONG;
                         } else {
-                            _numberBigInt = _bigNegative(l);
+                            BigInteger unsignedBase = _bigPositive(l);
+                            _numberBigInt = unsignedBase.negate().subtract(BigInteger.ONE);
                             _numTypesValid = NR_BIGINT;
                         }
                     }
@@ -1158,7 +1160,8 @@ public String nextTextValue() throws IOException
                     {
                         int v = _decode32Bits();
                         if (v < 0) {
-                            _numberLong = ((long) v) + -1L;
+                            long unsignedBase = (long) v & 0xFFFFFFFFL;
+                            _numberLong = -unsignedBase - 1L;
                             _numTypesValid = NR_LONG;
                         } else {
                             _numberInt = -v - 1;
@@ -1173,7 +1176,8 @@ public String nextTextValue() throws IOException
                         _numberLong = l;
                         _numTypesValid = NR_LONG;
                     } else {
-                        _numberBigInt = _bigNegative(l);
+                        BigInteger unsignedBase = _bigPositive(l);
+                        _numberBigInt = unsignedBase.negate().subtract(BigInteger.ONE);
                         _numTypesValid = NR_BIGINT;
                     }
                 }

diff --git a/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/ParserNumbersTest.java b/cbor/src/test/java/com/fasterxml/jackson/dataformat/cbor/parse/ParserNumbersTest.java
@@ -21,7 +21,7 @@
 public class ParserNumbersTest extends CBORTestBase
 {
     private final CBORFactory CBOR_F = cborFactory();
-    
+
     public void testIntValues() throws Exception
     {
         // first, single-byte
@@ -101,6 +101,17 @@ public void testInt32Overflow() throws Exception
         assertEquals(exp, p.getLongValue());
         assertEquals(NumberType.LONG, p.getNumberType());
         p.close();
+
+        // and, combined, a negative number where the mantissa overflows a signed int32
+        input = new byte[] {
+                (byte) CBORConstants.PREFIX_TYPE_INT_NEG + 26, // int32, that is, 4 more bytes
+                -1, -1, -1, -1
+        };
+        p = cborParser(input);
+        assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
+        assertEquals(-1L - 0xFFFFFFFFL, p.getLongValue());
+        assertEquals(NumberType.LONG, p.getNumberType());
+        p.close();
     }
 
     public void testLongValues() throws Exception
@@ -170,6 +181,21 @@ public void testInt64Overflow() throws Exception
         assertEquals(exp, p.getBigIntegerValue());
         assertEquals(NumberType.BIG_INTEGER, p.getNumberType());
         p.close();
+
+        // and, combined, a negative number where the mantissa overflows a signed int32
+        input = new byte[] {
+                (byte) CBORConstants.PREFIX_TYPE_INT_NEG + 27, // int32, that is, 4 more bytes
+                -1, -1, -1, -1, -1, -1, -1, -1
+        };
+        p = cborParser(input);
+        assertToken(JsonToken.VALUE_NUMBER_INT, p.nextToken());
+        exp = BigInteger.valueOf(Long.MAX_VALUE).shiftLeft(1)
+                .add(BigInteger.ONE)
+                .negate()
+                .subtract(BigInteger.ONE);
+        assertEquals(exp, p.getBigIntegerValue());
+        assertEquals(NumberType.BIG_INTEGER, p.getNumberType());
+        p.close();
     }
 
     public void testDoubleValues() throws Exception
@@ -198,15 +224,15 @@ private void _verifyDouble(CBORFactory f, double value, boolean isNaN) throws Ex
         assertEquals((float) value, p.getFloatValue());
 
         assertNull(p.nextToken());
-        
+
         // also skip
         p = cborParser(f, out.toByteArray());
         assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
         assertNull(p.nextToken());
-        
+
         p.close();
     }
-    
+
     public void testFloatValues() throws Exception
     {
         // first, single-byte
@@ -251,7 +277,7 @@ private void _verifyFloat(CBORFactory f, double value, boolean isNaN) throws Exc
         p = cborParser(f, out.toByteArray());
         assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
         assertNull(p.nextToken());
-        
+
         p.close();
     }
 
@@ -263,7 +289,7 @@ private void _verifyHalfFloat(JsonFactory f, int i16, double value) throws IOExc
         };
 
         boolean expNaN = Double.isNaN(value) || Double.isInfinite(value);
-        
+
         JsonParser p = f.createParser(data);
         assertEquals(JsonToken.VALUE_NUMBER_FLOAT, p.nextToken());
         assertEquals(expNaN, p.isNaN());