FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
310 stars 133 forks source link

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

Closed arthurscchan closed 9 months ago

arthurscchan commented 9 months ago

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 commented 9 months ago

I'm from the team that maintains https://github.com/amazon-ion/ion-java. IonReader.getIntegerSize() should never throw NullPointerException. I've opened https://github.com/amazon-ion/ion-java/issues/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.