Fix infinite loop due to integer overflow when reading large strings (#1352)

Author: pjfanning

release-notes/CREDITS-2.x

@@ -433,4 +433,9 @@ Antonin Janec (@xtonic)
  * Contributed #1217: Optimize char comparison using bitwise OR
   (2.17.0)
  * Contributed #1218: Simplify Unicode surrogate pair conversion for generation
- (2.17.0)
+  (2.17.0)
+
+Adam J. Shook (@adamjshook)
+ * Reported, suggested fix for  #1352: Fix infinite loop due to integer overflow
+   when reading large strings
+  (2.17.3)

release-notes/VERSION-2.x

@@ -20,6 +20,10 @@ a pure JSON library.
  (contributed by @pjfanning)
 #1340: Missing `JsonFactory` "provides" SPI with JPMS in `jackson-core` module
  (contributed by @sdyura)
+#1352: Fix infinite loop due to integer overflow when reading large strings
+ (reported by Adam J.S)
+ (fix contributed by @pjfanning)
+ 
 
 2.17.2 (05-Jul-2024)
 

src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java

@@ -2544,7 +2544,9 @@ private final void _finishString2(char[] outBuf, int outPtr)
                     outBuf = _textBuffer.finishCurrentSegment();
                     outPtr = 0;
                 }
-                final int max = Math.min(_inputEnd, (ptr + (outBuf.length - outPtr)));
+                final int max = Math.min(
+                    _inputEnd,
+                    InternalJacksonUtil.addOverflowSafe(ptr, outBuf.length - outPtr));
                 while (ptr < max) {
                     c = inputBuffer[ptr++] & 0xFF;
                     if (codes[c] != 0) {
@@ -2776,7 +2778,8 @@ protected JsonToken _handleApos() throws IOException
                 }
                 int max = _inputEnd;
                 {
-                    int max2 = _inputPtr + (outBuf.length - outPtr);
+                    final int max2 =
+                        InternalJacksonUtil.addOverflowSafe(_inputPtr, outBuf.length - outPtr);
                     if (max2 < max) {
                         max = max2;
                     }

src/main/java/com/fasterxml/jackson/core/json/async/NonBlockingUtf8JsonParserBase.java

@@ -6,6 +6,7 @@
 import com.fasterxml.jackson.core.io.IOContext;
 import com.fasterxml.jackson.core.json.JsonReadFeature;
 import com.fasterxml.jackson.core.sym.ByteQuadsCanonicalizer;
+import com.fasterxml.jackson.core.util.InternalJacksonUtil;
 import com.fasterxml.jackson.core.util.VersionUtil;
 
 import java.io.IOException;
@@ -2554,7 +2555,9 @@ private final JsonToken _finishRegularString() throws IOException
                     outBuf = _textBuffer.finishCurrentSegment();
                     outPtr = 0;
                 }
-                final int max = Math.min(_inputEnd, (ptr + (outBuf.length - outPtr)));
+                final int max = Math.min(
+                    _inputEnd,
+                    InternalJacksonUtil.addOverflowSafe(ptr, outBuf.length - outPtr));
                 while (ptr < max) {
                     c = getByteFromBuffer(ptr++) & 0xFF;
                     if (codes[c] != 0) {
@@ -2677,7 +2680,9 @@ private final JsonToken _finishAposString() throws IOException
                     outBuf = _textBuffer.finishCurrentSegment();
                     outPtr = 0;
                 }
-                final int max = Math.min(_inputEnd, (ptr + (outBuf.length - outPtr)));
+                final int max = Math.min(
+                    _inputEnd,
+                    InternalJacksonUtil.addOverflowSafe(ptr, outBuf.length - outPtr));
                 while (ptr < max) {
                     c = getByteFromBuffer(ptr++) & 0xFF;
                     if ((codes[c] != 0) && (c != INT_QUOTE)) {

src/main/java/com/fasterxml/jackson/core/util/InternalJacksonUtil.java

@@ -0,0 +1,25 @@
+package com.fasterxml.jackson.core.util;
+
+/**
+ * Internal Use Only. Helper class used to contain some useful utility methods.
+ *
+ * @since 2.17.3 / 2.18.1
+ */
+public abstract class InternalJacksonUtil {
+    /**
+     * Internal Use Only.
+     * <p>
+     * Method that will add two non-negative integers, and if result overflows, return
+     * {@link Integer#MAX_VALUE}. For performance reasons, does NOT check for
+     * the result being less than {@link Integer#MIN_VALUE}, nor whether arguments
+     * are actually non-negative.
+     * This is usually used to implement overflow-safe bounds checking.
+     */
+    public static int addOverflowSafe(final int base, final int length) {
+        int result = base + length;
+        if (result < 0) {
+             return Integer.MAX_VALUE;
+        }
+        return result;
+    }
+}

src/test/java/com/fasterxml/jackson/core/read/TestReadHumongousString.java

@@ -0,0 +1,55 @@
+package com.fasterxml.jackson.core.read;
+
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import com.fasterxml.jackson.core.JUnit5TestBase;
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.core.StreamReadConstraints;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
+
+// https://github.com/FasterXML/jackson-core/pull/1352
+class TestReadHumongousString extends JUnit5TestBase
+{
+    // disabled because it takes too much memory to run
+    @Disabled
+    // Since we might get infinite loop:
+    @Timeout(value = 10, unit = TimeUnit.SECONDS, threadMode = Timeout.ThreadMode.SEPARATE_THREAD)
+    @Test
+    void testLargeStringDeserialization() throws Exception {
+        final int len = Integer.MAX_VALUE - 1024;
+        final byte[] largeByteString = makeLongByteString(len);
+        final JsonFactory f = JsonFactory.builder()
+                .streamReadConstraints(StreamReadConstraints.builder()
+                        .maxStringLength(Integer.MAX_VALUE)
+                        .build())
+                .build();
+
+        try (JsonParser parser = f.createParser(largeByteString)) {
+            assertToken(JsonToken.VALUE_STRING, parser.nextToken());
+            // Let's not construct String but just check that length is
+            // expected: this avoids having to allocate 4 gig more of heap
+            // for test -- should still trigger problem if fix not valid
+            assertEquals(len, parser.getTextLength());
+            // TODO: could use streaming accessor (`JsonParser.getText(Writer)`)
+            assertNull(parser.nextToken());
+        }
+
+    }
+
+    private byte[] makeLongByteString(int length) {
+        final byte[] result = new byte[length + 2];
+        Arrays.fill(result, (byte) 'a');
+        result[0] = '\"';
+        result[length + 1] = '\"';
+        return result;
+    }
+}