NDPI: correct for potential integer overflow in stored restart marker tag #4083
Conversation
There was a problem hiding this comment.
Tested using one of the samples NDPI provided as part of the image.sc thread. Confirmed that without this change, the reading of the lower part of the image is corrupted but with the proposed changes it reads without issue.
In general, the proposed fix is consistent with the rest of the NDPI handling for large files and offsets exceeding 32-bit. While looking at other implementations, I came across openslide/openslide#276 which makes use of the private TIFF tag 65432 to add correct the marker value when it exceeds 2^32. Using the sample files, below are the values of the array element around the transition:
65426: 4294893067 65432: 0
65426: 4294899513 65432: 0
65426: 4294905822 65432: 0
65426: 4294911918 65432: 0
65426: 4294918072 65432: 0
65426: 4294924386 65432: 0
65426: 4294930891 65432: 0
65426: 4294937018 65432: 0
65426: 4294943199 65432: 0
65426: 4294949427 65432: 0
65426: 4294955562 65432: 0
65426: 4294961725 65432: 0
65426: 597 65432: 1
65426: 6829 65432: 1
65426: 12992 65432: 1
65426: 19166 65432: 1
65426: 25370 65432: 1
65426: 31662 65432: 1
65426: 37844 65432: 1
65426: 44021 65432: 1
65426: 50178 65432: 1
65426: 56342 65432: 1
65426: 62539 65432: 1
65426: 68702 65432: 1
65426: 74838 65432: 1
65426: 80984 65432: 1
65426: 87163 65432: 1
65426: 93372 65432: 1
65426: 99511 65432: 1
65426: 105642 65432: 1
@melissalinkert how do you feel about using this alternate approach to correct marker values? The main downside is that it might not necessarily apply for all NDPI files but we could test that against our curated QA samples
|
3bb4a80 checks for the tag containing the high bytes of the marker offsets. If that tag is missing and the file is larger than 4GB, the marker offset calculation falls back to adding 2^32 when it's clear that the offsets have overflowed. It looks like the tag is present for the full-resolution IFD, but not (necessarily?) for sub-resolutions. |
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/unreadable-big-ome-tiff-on-omero/89541/2 |
Fixes #4082.
Use the files in
curated/hamamatsu/imagesc-84874/. Without this PR,showinf -noflatwith a-cropoption that picks the bottom ~1000 pixels (any X coordinate and tile width) should show corrupted image data. With this PR, the same test should show a smooth image with no obvious corruption.