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

TIFF: prevent integer overflow when reading from a large tile #3519

Merged
merged 2 commits into from
Mar 10, 2020

Conversation

melissalinkert
Copy link
Member

When reading from a single tile whose byte count is > 2GB,
use seek instead of skipBytes to prevent a possible integer overflow.

/cc @chris-allan, @douglasrennick

When reading from a single tile whose byte count is > 2GB,
use "seek" instead of "skipBytes" to prevent a possible integer overflow.
@chris-allan
Copy link
Member

As a matter of background, we were seeing very weird behaviour trying to do large pyramid conversions from multi-GB TIFFs with no internal pyramids.

/cc @seb, @dgault

@@ -1026,7 +1026,9 @@ else if (stripByteCounts[countIndex] < 0 && countIndex > 0) {
// we only want a piece of the tile, so read each row separately
// this is especially necessary for large single-tile images
int bpp = bytes * effectiveChannels;
in.skipBytes((int) (y * bpp * tileWidth));
// don't use skipBytes here, as the number of bytes to skip may
// be greater than Integer.MAX_VALUE if the tile is large
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just use skipBytes(long).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about that API method; fixed in b47dafa.

@melissalinkert
Copy link
Member Author

An artificial test file is now in inbox/gh-3519/. The test file contains a gradient from black at the top to white at the bottom.

To see the problem without this PR:

$ bfconvert -tilex 1024 -tiley 1024 -bigtiff -pyramid-scale 2 -pyramid-resolutions 4 large-gradient.tiff bfconvert.ome.tiff
$ showinf -noflat -resolution 3 bfconvert.ome.tiff

The top portion of the image should show a gradient as expected, but the bottom portion will be oddly striped. With this PR, the same test should result in an image with a completely smooth gradient.

@dgault dgault added this to the 6.4.0 milestone Mar 10, 2020
@dgault
Copy link
Member

dgault commented Mar 10, 2020

Using the new sample file the behaviour can be easily reproduced and is visible across the different resolution levels. I couldn't find any obvious pattern in the starting point of the corruption though.

Screen Shot 2020-03-10 at 22 18 09

With the PR included the image is read and displayed without andy issues. There have been no regressions in any tests, merging this to included for 6.4.0

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 this pull request may close these issues.

4 participants