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

Unexpected NullPointerException thrown from IonParser::getNumberType() #434

Closed
arthurscchan opened this issue Dec 27, 2023 · 1 comment · Fixed by #435
Closed

Unexpected NullPointerException thrown from IonParser::getNumberType() #434

arthurscchan opened this issue Dec 27, 2023 · 1 comment · Fixed by #435
Labels
2.17 fuzz Issue found by OssFuzz ion
Milestone

Comments

@arthurscchan
Copy link
Contributor

arthurscchan commented Dec 27, 2023

In the IonParser::getNumberType() method, there is an invocation of the IonReader.getIntegerSize() method which could return a null value in some cases with invalid data. If the result is null, the code will throw a NullPointerException in the next line when the value is used for the switch condition.

Also, IonReader.getIntegerSize() method will throw NullPointerException in some cases, thus it is also necessary to wrap around the method invocation to ensure NullPointerException is caught.

    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) {
...

The suggested fix is to add a null checking after the invocation of the IonReader.getIntegerSize() method and throw an exception if the return value stored in size is indeed null. Also, wrap the IonReader.getIntegerSize() method invocation with a try catch block to catch the possible NullPointerException.

We found this issue by OSS-Fuzz and it is reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65268 and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65274.

@tgregg
Copy link
Contributor

tgregg commented Dec 27, 2023

I'm from the team that maintains https://github.com/amazon-ion/ion-java. IonReader.getIntegerSize() should never throw NullPointerException. I've opened amazon-ion/ion-java#685 to get that fixed.

It will return null when the reader is not positioned on an integer value, so that needs to be handled as described above.

@cowtowncoder cowtowncoder changed the title Unexpected NullPointerException thrown from IonParser::getNumberType() Unexpected NullPointerException thrown from IonParser::getNumberType() Dec 30, 2023
@cowtowncoder cowtowncoder added ion 2.17 fuzz Issue found by OssFuzz labels Dec 30, 2023
@cowtowncoder cowtowncoder added this to the 2.17.0 milestone Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 fuzz Issue found by OssFuzz ion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants