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

Large ignored chunk causes crash #3

Open
MarkJeronimus opened this issue Jul 22, 2017 · 3 comments
Open

Large ignored chunk causes crash #3

MarkJeronimus opened this issue Jul 22, 2017 · 3 comments

Comments

@MarkJeronimus
Copy link

MarkJeronimus commented Jul 22, 2017

When there is a large ignored chunk, it crashes in production, but not when loaded with PngInfo.java

Analysis of the cause:
The offending chunk starts decoding in DefaultPngChunkReader.readChunk(). It calls DefaultPngChunkReader.readOtherChunk() which eventually calls (via the filter input stream) BufferedInputStream.skip().

From the Javadoc of skip():

Skips over and discards n bytes of data from the input stream. The skip method may, for a variety of reasons, end up skipping over some smaller number of bytes, possibly 0. The actual number of bytes skipped is returned.

And indeed, it skips up to available() bytes, which is just under 8192 because the current pointer is still small and that's the buffer's size.

PngInfo.java doesn't wrap the input stream in a BufferedInputStream so it doesn't expose the bug.

Test image: iTXt chunk (3rd ignored chunk) of this (NSFW) image (navigate to the "Original" link in the left column!)

@MarkJeronimus
Copy link
Author

Fixed.

public void readOtherChunk(int code, PngSource source, int dataPosition, int dataLength) throws IOException {
  // If we're not processing it, got to skip it.
  while (dataLength > 0) {
      dataLength -= source.skip(dataLength);
  }

}

@MarkJeronimus MarkJeronimus changed the title Too big ignored chunk causes crash Large ignored chunk causes crash Jul 22, 2017
@aellerton
Copy link
Owner

Thanks for this @MarkJeronimus!

@MarkJeronimus
Copy link
Author

I found other places too where skip() is being used. You'd better check them all if you're still working on this project.

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

No branches or pull requests

2 participants