Skip to content

Commit

Permalink
Fix #834 (rare buffer condition for number-parsing, found by oss-fuzz…
Browse files Browse the repository at this point in the history
… project)
  • Loading branch information
cowtowncoder committed Oct 30, 2022
1 parent 2e2cff0 commit 0d48020
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 3 deletions.
4 changes: 4 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ JSON library.
=== Releases ===
------------------------------------------------------------------------

Not yet released:

#834: ReaderBaseJsonParser._verifyRootSpace() can cause buffer boundary failure

2.14.0-rc3 (28-Oct-2022)
2.14.0-rc2 (10-Oct-2022)
2.14.0-rc1 (25-Sep-2022)
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/fasterxml/jackson/core/JsonEncoding.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public enum JsonEncoding {
UTF32_BE("UTF-32BE", true, 32),
UTF32_LE("UTF-32LE", false, 32)
;

private final String _javaName;

private final boolean _bigEndian;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,13 @@ private final void _verifyRootSpace(int ch) throws IOException
case '\t':
return;
case '\r':
_skipCR();
// 29-Oct-2022, tatu: [core#834] requires change here, we MUST NOT
// force a read. As such let's simply push back the \r without
// further ado; it is enough to know there is valid WS separating
// NOTE: may need to revisit handling of plain \n to keep Location
// info more uniform. But has to do for now.
// _skipCR();
--_inputPtr;
return;
case '\n':
++_currInputRow;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,10 @@ private final void _verifyRootSpace(int ch) throws IOException
case '\t':
return;
case '\r':
_skipCR();
// 29-Oct-2022, tatu: [core#834] While issue is only relevant for char-backed
// sources, let's unify handling to keep behavior uniform.
// _skipCR();
--_inputPtr;
return;
case '\n':
++_currInputRow;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.fasterxml.jackson.core.fuzz;

import java.io.*;
import java.math.BigInteger;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.core.exc.StreamReadException;
import com.fasterxml.jackson.core.testsupport.ThrottledInputStream;

// Reproducing: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52688
// (reported as [core#834]
public class Fuzz52688ParseTest extends BaseTest
{
private final JsonFactory JSON_F = new JsonFactory();

private final static BigInteger BIG_NUMBER = new BigInteger("3222"
+"2222"
+"2222"
+"2222"
+"222");

public void testBigNumberUTF16Parse() throws Exception
{
// 41 bytes as UTF16-LE; becomes 21 characters (last broken)
final byte[] DOC = {
0x33, 0, 0x32, 0, 0x32, 0, 0x32, 0,
0x32, 0, 0x32, 0, 0x32, 0, 0x32, 0,
0x32, 0, 0x32, 0, 0x32, 0, 0x32, 0,
0x32, 0, 0x32, 0, 0x32, 0, 0x32, 0,
0x32, 0, 0x32, 0, 0x32, 0, 0xd, 0,
0x32
};

try (JsonParser p = JSON_F.createParser(/*ObjectReadContext.empty(), */
new ByteArrayInputStream(DOC))) {
assertEquals(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(BIG_NUMBER, p.getBigIntegerValue());
assertEquals(1, p.currentLocation().getLineNr());

// and now we should fail for the weird character
try {
JsonToken t = p.nextToken();
fail("Should not pass, next token = "+t);
} catch (StreamReadException e) {
verifyException(e, "Unexpected character");
assertEquals(2, p.currentLocation().getLineNr());
assertEquals(2, e.getLocation().getLineNr());
}
}
}

public void testBigNumberUTF8Parse() throws Exception
{
// Similar to UTF-16 case
final byte[] DOC = {
0x33, 0x32, 0x32, 0x32,
0x32, 0x32, 0x32, 0x32,
0x32, 0x32, 0x32, 0x32,
0x32, 0x32, 0x32, 0x32,
0x32, 0x32, 0x32, 0xd,
(byte) '@'
};

// Try to force buffer condition
try (ThrottledInputStream in = new ThrottledInputStream(DOC, 1)) {
try (JsonParser p = JSON_F.createParser(/*ObjectReadContext.empty(), */ in)) {
assertEquals(JsonToken.VALUE_NUMBER_INT, p.nextToken());
assertEquals(BIG_NUMBER, p.getBigIntegerValue());
assertEquals(1, p.currentLocation().getLineNr());

// and now we should fail for the weird character
try {
JsonToken t = p.nextToken();
fail("Should not pass, next token = "+t);
} catch (StreamReadException e) {
verifyException(e, "Unexpected character");
assertEquals(2, p.currentLocation().getLineNr());
assertEquals(2, e.getLocation().getLineNr());
}
}
}
}
}

0 comments on commit 0d48020

Please sign in to comment.