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

ArrayIndexOutOfBoundsException from UTF32Reader.read on invalid input #382

Closed
emilyselwood opened this issue Jun 2, 2017 · 6 comments
Closed

Comments

@emilyselwood
Copy link
Contributor

We are doing some fuzz testing on internal projects and found some input that causes an unexpected exception from JsonParser. While it is throwing an exception and stopping on this, it should probably be some kind of IOException rather than an ArrayIndexOutOfBoundsException.

In our case we are catching the three exception types listed in ObjectMapper.readTree (JsonParseException | JsonProcessingException | IOException) and this managed to escape.

Simple test case follows

public void testInvalidInput() throws IOException {

        byte[] data = {
                0x00,
                0x00,
                0x00,
                0x20,
                (byte) 0xFE,
                (byte) 0xFF,
                0x00,
                0x01,
                (byte) 0xFB
        };

        JsonFactory FACTORY = new JsonFactory();
        JsonParser parser = FACTORY.createParser(data);

        parser.nextToken();


    }

stack trace:

java.lang.ArrayIndexOutOfBoundsException: 9

	at com.fasterxml.jackson.core.io.UTF32Reader.read(UTF32Reader.java:138)
	at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._loadMore(ReaderBasedJsonParser.java:243)
	at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._skipWSOrEnd(ReaderBasedJsonParser.java:2331)
	at com.fasterxml.jackson.core.json.ReaderBasedJsonParser.nextToken(ReaderBasedJsonParser.java:646)
	at com.fasterxml.jackson.core.io.TestUTF32Reader.testInvalidInput(TestUTF32Reader.java:32)
@cowtowncoder
Copy link
Member

Thank you for reporting this. It probably doesn't come as a big surprise, but usage for UTF-32 is rather low, and as such we don't often get bug reports, compared to UTF-8. And existing testing is bit limited (there is some but only for simplest use cases).

@cowtowncoder
Copy link
Member

Does indeed look like input is invalid. But I agree it should be handled in proper manner as an IOException or subtypes (I'll see how other Unicode decoding problems are reported).

@emilyselwood
Copy link
Contributor Author

Indeed I'm not surprised. Most of our normal valid input is UTF-8. As I said this one popped out of a fuzz test. Thought I would provide a patch as it seemed reasonably simple to deal with using the existing unexpected end of file functionality.

@cowtowncoder
Copy link
Member

@wselwood yes, much appreciated!

@cowtowncoder cowtowncoder modified the milestones: 2.2.1, 2.8.9 Jun 2, 2017
@cowtowncoder
Copy link
Member

@wselwood Thank you for reporting this, providing fix -- I ended up merging manually, using test, partly for backporting, and partly as I wanted to change code in related parts. Fix will be in 2.8.9, 2.9.0(.pr4)

@emilyselwood
Copy link
Contributor Author

Fair enough @cowtowncoder . You did a much better job of the fix and tests than I did. Thanks for the quick work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants