Skip to content

Commit

Permalink
Fix issue in handling of corrupt Ion number (#429)
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder authored Dec 22, 2023
1 parent fc337c4 commit 8039f96
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,22 @@ public int getTextOffset() throws IOException {
@Override
public BigInteger getBigIntegerValue() throws IOException {
_verifyIsNumberToken();
return _reader.bigIntegerValue();
try {
return _reader.bigIntegerValue();
} catch (IonException e) {
return _reportCorruptNumber(e);
}
}

@Override
public BigDecimal getDecimalValue() throws IOException {

_verifyIsNumberToken();
return _reader.bigDecimalValue();
try {
return _reader.bigDecimalValue();
} catch (IonException e) {
return _reportCorruptNumber(e);
}
}

@Override
Expand Down Expand Up @@ -566,13 +575,10 @@ public JsonToken nextToken() throws IOException
try {
type = _reader.next();
} catch (IonException e) {
_wrapError(e.getMessage(), e);

// [dataformats-binary#420]: IonJava leaks IOOBEs so:
return _reportCorruptContent(e);
// [dataformats-binary#420]: IonJava leaks IOOBEs so:
} catch (IndexOutOfBoundsException e) {
_wrapError(String.format("Corrupt content to decode; underlying failure: (%s) %s",
e.getClass().getName(), e.getMessage()),
e);
return _reportCorruptContent(e);
}
if (type == null) {
if (_parsingContext.inRoot()) { // EOF?
Expand All @@ -589,13 +595,15 @@ public JsonToken nextToken() throws IOException
boolean inStruct = !_parsingContext.inRoot() && _reader.isInStruct();
// (isInStruct can return true for the first value read if the reader
// was created from an IonValue that has a parent container)
final String name;
try {
// getFieldName() can throw an UnknownSymbolException if the text of the
// field name symbol cannot be resolved.
_parsingContext.setCurrentName(inStruct ? _reader.getFieldName() : null);
} catch (UnknownSymbolException e) {
_wrapError(e.getMessage(), e);
name = inStruct ? _reader.getFieldName() : null;
} catch (IonException e) {
return _reportCorruptContent(e);
}
_parsingContext.setCurrentName(name);
JsonToken t = _tokenFromType(type);
// and return either field name first
if (inStruct) {
Expand Down Expand Up @@ -709,6 +717,20 @@ protected void _handleEOF() throws JsonParseException
}
}

private <T> T _reportCorruptContent(Exception e) throws IOException
{
final String msg = String.format("Corrupt content to decode; underlying failure: (%s) %s",
e.getClass().getName(), e.getMessage());
throw _constructError(msg, e);
}

private <T> T _reportCorruptNumber(Exception e) throws IOException
{
final String msg = String.format("Corrupt Number value to decode; underlying failure: (%s) %s",
e.getClass().getName(), e.getMessage());
throw _constructError(msg, e);
}

@Override
public void overrideCurrentName(String name) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,25 @@
// [dataformats-binary#424]
public class Fuzz424_65065_65126NPETest
{
private final IonObjectMapper MAPPER = IonObjectMapper.builder().build();

@Test
public void testFuzz65065() throws Exception {
IonFactory f = IonFactory
.builderForBinaryWriters()
.build();

IonObjectMapper mapper = IonObjectMapper.builder(f).build();

try {
byte[] bytes = {(byte) -32, (byte) 1, (byte) 0, (byte) -22, (byte) 123, (byte) -112};
mapper.readTree(f.createParser(new ByteArrayInputStream(bytes)));
MAPPER.readTree(new ByteArrayInputStream(bytes));
fail("Should not pass (invalid content)");
} catch (StreamReadException e) {
e.printStackTrace();
assertThat(e.getMessage(), Matchers.containsString("Internal `IonReader` error"));
}
}

@Test
public void testFuzz65126() throws Exception {
IonFactory f = IonFactory
.builderForBinaryWriters()
.build();

try {
byte[] bytes = {(byte) 1, (byte) 0};
f.createParser(bytes).getDecimalValue();
MAPPER.createParser(bytes).getDecimalValue();
fail("Should not pass (invalid content)");
} catch (StreamReadException e) {
assertThat(e.getMessage(), Matchers.containsString("Current token (null) not numeric"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.fasterxml.jackson.dataformat.ion.fuzz;

import java.io.InputStream;

import org.hamcrest.Matchers;
import org.junit.Test;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.core.exc.StreamReadException;
import com.fasterxml.jackson.dataformat.ion.*;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

// https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65062
public class Fuzz_65062_VarintTest
{
final IonObjectMapper MAPPER = IonObjectMapper.builder().build();

@Test
public void testFuzz65062_Varint() throws Exception {
try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65062.ion")) {
try (JsonParser p = MAPPER.createParser(in)) {
assertEquals(JsonToken.START_ARRAY, p.nextToken());

while (p.nextToken() == JsonToken.VALUE_NUMBER_FLOAT) {
p.getDecimalValue();
}
assertEquals(JsonToken.END_ARRAY, p.nextToken());
}
fail("Should not pass (invalid content)");
} catch (StreamReadException e) {
// 21-Dec-2023, tatu: Not 100% sure why we won't get Number-specific fail but:
assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode; underlying failure"));
}
}
}
Binary file added ion/src/test/resources/data/fuzz-65062.ion
Binary file not shown.

0 comments on commit 8039f96

Please sign in to comment.