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

IonReader throws NullPointerException for unchecked invalid data #424

Closed
arthurscchan opened this issue Dec 18, 2023 · 0 comments
Closed
Milestone

Comments

@arthurscchan
Copy link
Contributor

In the IonParser::getText() method, there is a call to the IonReader::stringValue(). Also, in IonParser::getXXXValue() for retrieving different number values from the IonReader calls to underlying IonReader for retrieving string or number value. According to the Javadoc of IonReader, each of the APIs requires a special IonType and IllegalStateException could be thrown if the wrong type is passed. But there is a special case when there is no more input, the IonType will be null and continuing calling those methods will result in NullPointerException.

    @Override
    public String getText() throws IOException
    {
         if (_currToken != null) { // null only before/after document
          ......
            case VALUE_STRING:
                try {
                    // stringValue() will throw an UnknownSymbolException if we're
                    // trying to get the text for a symbol ID that cannot be resolved.
                    return _reader.stringValue();
                } catch (UnknownSymbolException e) {
                    throw _constructError(e.getMessage(), e);
                }
            ......           
        return null;
    }
...
    @Override
    public BigInteger getBigIntegerValue() throws IOException {
        return _reader.bigIntegerValue();
    }

    @Override
    public BigDecimal getDecimalValue() throws IOException {
        return _reader.bigDecimalValue();
    }

    @Override
    public double getDoubleValue() throws IOException {
        return _reader.doubleValue();
    }

    @Override
    public float getFloatValue() throws IOException {
        return (float) _reader.doubleValue();
    }

    @Override
    public int getIntValue() throws IOException {
        return _reader.intValue();
    }

    @Override
    public long getLongValue() throws IOException {
        return _reader.longValue();
    }

It is found that in the IonParser::getNumberValue() method, there is a null check to ensure the IonType (and NumberType) of the current token is not null before calling the corresponding data retrieving method in the IonReader implementation. But these null checks are missing from the above method which could cause unexpected NullPointerException.

    @Override
    public Number getNumberValue() throws IOException {
        NumberType nt = getNumberType();
        if (nt != null) {
            switch (nt) {
            case INT:
                return _reader.intValue();
            case LONG:
                return _reader.longValue();
            case FLOAT:
                return (float) _reader.doubleValue();
            case DOUBLE:
                return _reader.doubleValue();
            case BIG_DECIMAL:
                return _reader.bigDecimalValue();
            case BIG_INTEGER:
                return getBigIntegerValue();
            }
        }
        return null;
    }

The simplest fix is to add a null check similar to the one done in the IonParser::getNumberValue() method.

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

@cowtowncoder cowtowncoder changed the title IonReader throws NullPointerException for unchecked invalid data IonReader throws NullPointerException for unchecked invalid data Dec 19, 2023
@cowtowncoder cowtowncoder added this to the 2.17.0 milestone Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants