jackson-dataformats-binary icon indicating copy to clipboard operation
jackson-dataformats-binary copied to clipboard

`NullPointerException` in `IonParser.nextToken()`

Open ZanderHuang opened this issue 2 years ago • 4 comments

Description

This vulnerability is of Uncaught Exception for java.lang.NullPointerException in com.fasterxml.jackson.dataformat, jackson-dataformat-ion (2.13.0, the latest version) with com.amazon.ion, ion-java (1.8.3, the latest version). Specifically, it fails to check the runtime exception java.lang.NullPointerException in function com.fasterxml.jackson.dataformat.ion.IonParser.nextToken() ( IonParser.java: 506 ).
The attackers can launch DoS (Denial of Service) attacks to any program that directly uses this library (CWE-2248: Uncaught exception).

The vulnerable code:

        // the _reader.next() can throw java.lang.NullPointerException
        IonType type = _reader.next();
        if (type == null) {
        ...

The crash stack:

com.amazon.ion.impl.LocalSymbolTable.readOneImport::LocalSymbolTable.java:681
com.amazon.ion.impl.LocalSymbolTable.prepImportsList::LocalSymbolTable.java:646
com.amazon.ion.impl.LocalSymbolTable.readLocalSymbolTable::LocalSymbolTable.java:304
com.amazon.ion.impl.LocalSymbolTable$Factory.newLocalSymtab::LocalSymbolTable.java:68
com.amazon.ion.impl.IonReaderBinaryUserX.has_next_helper_user::IonReaderBinaryUserX.java:252
com.amazon.ion.impl.IonReaderBinaryUserX.hasNext::IonReaderBinaryUserX.java:222
com.amazon.ion.impl.IonReaderBinaryUserX.next::IonReaderBinaryUserX.java:208
com.fasterxml.jackson.dataformat.ion.IonParser.nextToken::IonParser.java:506
com.fasterxml.jackson.databind.ObjectMapper._readTreeAndClose::ObjectMapper.java:4649
com.fasterxml.jackson.databind.ObjectMapper.readTree::ObjectMapper.java:3074
com.test.Entry.main::Entry.java:51

Proof of Concept

  • download the program that uses jackson and built it
cd bug_reproduce_program_jackson_ion
bash build.sh
  • use one of the poc to trigger the crash
java -jar built-target-program.jar pocfile

Fix suggestion

Wrap this kind of exception as a type of exception the library provided, e.g. IonException. Maybe the fix should not only in jackson but also in its dependent ion-java package.

Impact

The attackers can launch DoS (Denial of Service) attacks to any program that directly uses this library (CWE-2248: Uncaught exception).

ZanderHuang avatar Nov 01 '21 16:11 ZanderHuang

Thank you for reporting this issue: sounds like sub-optimal handling.

I am not sure I see DoS aspect itself as exceptions are the mechanism to use for many kinds of invalid data, but in this case handling should produce package-specified exception, not accidental NPE.

cowtowncoder avatar Nov 01 '21 19:11 cowtowncoder

I added a failing unit test for this one, but I do think actual fix needs to go in ion-java; Jackson cannot prevent it from being thrown and so any performance problems are already incurred even if caught by IonParser -- and adding NPE catching in there just seems incorrect to me.

@mcliedtke @jobarr-amzn do you know what'd be a good way to report this to streaming Ion codec? Test is UncaughtException303Test and uses a small broken Ion doc from under ion/src/test/resources//data/issue-303.ion to trigger NPE.

cowtowncoder avatar Nov 02 '21 03:11 cowtowncoder

I'm taking a look- will open an issue in ion-java as necessary.

jobarr-amzn avatar Nov 04 '21 18:11 jobarr-amzn

Hi @jobarr-amzn! I assume you haven't had a chance to look into this but thought I'd ping just in case.

cowtowncoder avatar Apr 29 '22 03:04 cowtowncoder