Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #434: Add null check before accessing integer size #435

Merged
merged 8 commits into from
Dec 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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