Skip to content

Commit

Permalink
Fix infinite loop due to integer overflow when reading large strings (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
pjfanning authored Oct 28, 2024
1 parent 6ce1c0d commit 7db88c1
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 5 deletions.
7 changes: 6 additions & 1 deletion release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)
4 changes: 4 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}

0 comments on commit 7db88c1

Please sign in to comment.