From 7db88c1585fd56a59bd90b22c03e35d23f5a2724 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 28 Oct 2024 03:02:19 +0000 Subject: [PATCH] Fix infinite loop due to integer overflow when reading large strings (#1352) --- release-notes/CREDITS-2.x | 7 ++- release-notes/VERSION-2.x | 4 ++ .../core/json/UTF8StreamJsonParser.java | 7 ++- .../async/NonBlockingUtf8JsonParserBase.java | 9 ++- .../core/util/InternalJacksonUtil.java | 25 +++++++++ .../core/read/TestReadHumongousString.java | 55 +++++++++++++++++++ 6 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/fasterxml/jackson/core/util/InternalJacksonUtil.java create mode 100644 src/test/java/com/fasterxml/jackson/core/read/TestReadHumongousString.java diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index b60d60bb62..c419898abf 100644 --- a/release-notes/CREDITS-2.x +++ b/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) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index c4684c75d1..8088d98758 100644 --- a/release-notes/VERSION-2.x +++ b/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) diff --git a/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java b/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java index 62aa0ed7af..b45143777b 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java +++ b/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; } diff --git a/src/main/java/com/fasterxml/jackson/core/json/async/NonBlockingUtf8JsonParserBase.java b/src/main/java/com/fasterxml/jackson/core/json/async/NonBlockingUtf8JsonParserBase.java index 0f9594dce4..bbf4796635 100644 --- a/src/main/java/com/fasterxml/jackson/core/json/async/NonBlockingUtf8JsonParserBase.java +++ b/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)) { diff --git a/src/main/java/com/fasterxml/jackson/core/util/InternalJacksonUtil.java b/src/main/java/com/fasterxml/jackson/core/util/InternalJacksonUtil.java new file mode 100644 index 0000000000..4b356552b5 --- /dev/null +++ b/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. + *

+ * 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; + } +} diff --git a/src/test/java/com/fasterxml/jackson/core/read/TestReadHumongousString.java b/src/test/java/com/fasterxml/jackson/core/read/TestReadHumongousString.java new file mode 100644 index 0000000000..7158251373 --- /dev/null +++ b/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; + } +}