Skip to content

Commit

Permalink
Fixed #434: Add null check before accessing integer size (#435)
Browse files Browse the repository at this point in the history
  • Loading branch information
arthurscchan authored Dec 30, 2023
1 parent 80ae368 commit bf8d729
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -384,27 +384,47 @@ private void _verifyIsNumberToken() throws IOException
@Override
public NumberType getNumberType() throws IOException
{
IonType type = _reader.getType();
if (type != null) {
// Hmmh. Looks like Ion gives little bit looser definition here;
// harder to pin down exact type. But let's try some checks still.
switch (type) {
case DECIMAL:
//Ion decimals can be arbitrary precision, need to read as big decimal
return NumberType.BIG_DECIMAL;
case INT:
IntegerSize size = _reader.getIntegerSize();
switch (size) {
if (_currToken == JsonToken.VALUE_NUMBER_INT
|| _currToken == JsonToken.VALUE_NUMBER_FLOAT
// 30-Dec-2023, tatu: This is odd, but some current tests seem to
// expect this case to work when creating `IonParser` from `IonReader`,
// which does not seem to work without work-around like this:
|| ((_currToken == null) && !isClosed())) {
IonType type = _reader.getType();
if (type != null) {
// Hmmh. Looks like Ion gives little bit looser definition here;
// harder to pin down exact type. But let's try some checks still.
switch (type) {
case DECIMAL:
//Ion decimals can be arbitrary precision, need to read as big decimal
return NumberType.BIG_DECIMAL;
case INT:
return NumberType.INT;
case LONG:
return NumberType.LONG;
final IntegerSize size;
// [dataformats-binary#434]: another problem with corrupt data handling.
// Temporary measure until this bug fixing is merged and published
// https://github.com/amazon-ion/ion-java/issues/685
try {
size = _reader.getIntegerSize();
} catch (IonException e) {
return _reportCorruptNumber(e);
} catch (NullPointerException e) {
return _reportCorruptContent(e);
}
if (size == null) {
_reportError("Current token (%s) not integer", _currToken);
}
switch (size) {
case INT:
return NumberType.INT;
case LONG:
return NumberType.LONG;
default:
return NumberType.BIG_INTEGER;
}
case FLOAT:
return NumberType.DOUBLE;
default:
return NumberType.BIG_INTEGER;
}
case FLOAT:
return NumberType.DOUBLE;
default:
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ public void testFloatType() throws IOException {
reader.stepIn();
// Step next.
reader.next();
// 30-Dec-2023, tatu: This is problematic as created parser is expected
// to point to `JsonToken.VALUE_NUMBER_FLOAT`, but `createParser()`
// does not initialize state. For now, `IonParser.getNumberType()` has
// special handling allowing this case but that should not be needed
final IonParser floatParser = new IonFactory().createParser(reader);
Assert.assertEquals(JsonParser.NumberType.DOUBLE, floatParser.getNumberType());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package com.fasterxml.jackson.dataformat.ion.fuzz;

import java.io.*;

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.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.ion.*;

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

// [dataformats-binary#434]
public class Fuzz434_65268_65274_NPETest
{
private final ObjectMapper ION_MAPPER = new IonObjectMapper();

// Test that used to fail on "getNumberType()" for `JsonToken.VALUE_NULL`
@Test
public void testFuzz65268() throws Exception {
try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65268.ion")) {
try (JsonParser p = ION_MAPPER.createParser(in)) {
assertEquals(JsonToken.VALUE_STRING, p.nextToken());
p.getText();
assertNull(p.nextTextValue());
assertEquals(JsonToken.VALUE_NULL, p.currentToken());
assertNull(p.getNumberType());
}
}
}

@Test
public void testFuzz65274Malformed() throws Exception {
try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65274.ion")) {
byte[] invalid = new byte[in.available()];
new DataInputStream(in).readFully(invalid);
ION_MAPPER.readTree(new ByteArrayInputStream(invalid));
fail("Should not pass (invalid content)");
} catch (StreamReadException e) {
assertThat(e.getMessage(), Matchers.containsString("Corrupt content to decode"));
}
}

@Test
public void testFuzz65274Eof() throws Exception {
try (InputStream in = getClass().getResourceAsStream("/data/fuzz-65274.ion")) {
ION_MAPPER.readTree(in);
fail("Should not pass (invalid content)");
} catch (StreamReadException e) {
assertThat(e.getMessage(), Matchers.containsString("Corrupt Number value to decode"));
}
}
}
3 changes: 3 additions & 0 deletions ion/src/test/resources/data/fuzz-65268.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
n

null.int����~���ttttt(�t�740���������n����66}tt
Binary file added ion/src/test/resources/data/fuzz-65274.ion
Binary file not shown.
2 changes: 2 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,5 @@ Arthur Chan (@arthurscchan)
* Contributed #432 (ion) More methods from `IonReader` could throw an unexpected
`AssertionError`
(2.17.0)
* Contributed #434 (ion) Unexpected `NullPointerException` thrown from `IonParser::getNumberType()`
(2.17.0)
6 changes: 4 additions & 2 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ Active maintainers:
#424: (ion) `IonReader` throws `NullPointerException` for unchecked invalid data
(fix contributed by Arthur C)
#426: (smile) `SmileParser` throws unexpected IOOBE for corrupt content
(fix contributed by Arthur C)
(fix contributed by Arthur C)-(
#432: (ion) More methods from `IonReader` could throw an unexpected `AssertionError`
(fix contributed by Arthur C)
-(ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5)
#434 (ion) Unexpected `NullPointerException` thrown from `IonParser::getNumberType()`
(fix contributed by Arthur C)
- (ion) Update `com.amazon.ion:ion-java` to 1.11.0 (from 1.10.5)

2.16.1 (24-Dec-2023)

Expand Down

0 comments on commit bf8d729

Please sign in to comment.