fix(raw): fix undebayered flip mapping, buffer bounds, overflow, etc. - #5415
Open
lgritz wants to merge 1 commit into
Open
fix(raw): fix undebayered flip mapping, buffer bounds, overflow, etc.#5415lgritz wants to merge 1 commit into
lgritz wants to merge 1 commit into
Conversation
More auditing of the raw plugin turned up additional bugs. raw_bps is floored at 8 bits per sample. Some headers leave it 0, and Phase One stores a format code there instead. Either value collapsed declared_bytes to 0 and let the bomb guard evaporate. read_native_scanline's undebayered path (raw:Demosaic=none) mapped only 4 of LibRaw's 8 flip codes. Flip 1, 2, 4 and 7 fell through the if/else chain and returned true without writing the caller's buffer. That is an uninitialized-heap disclosure, not just wrong data. Flip 3 mirrored only vertically, disagreeing with the debayered path. The chain is replaced by the bitwise mapping flip_index() uses: bit 2 transposes, bit 1 mirrors rows, bit 0 mirrors columns. Flips 0, 5 and 6 are unchanged. The same path indexed rawdata.raw_image from header-derived extents alone. Offsets were computed in 32-bit int. The index is now computed in 64-bit and bounded against raw_width*raw_height, LibRaw's actual allocation. That bound uses the allocation, not the header geometry, because Fuji rotated-sensor images are wider than the sensor grid. The debayered path now checks LibRaw's returned channel count and scanline offset against the caller's spec and m_image->data_size before writing. do_unpack()'s return value was discarded at the call site. m_unpacked was set even when a post-unpack re-open failed. Both are now checked. exif_parser_cb multiplied tiff_data_size() by an unvalidated count and passed the product to std::vector. tiff_data_size() returns size_t(-1) for an unrecognized type, so the multiply could overflow and throw an unguarded std::length_error. Type, count and read return value are now validated. get_thumbnail leaked the libraw_processed_image_t from dcraw_make_mem_thumb on every path. It is now owned by a unique_ptr bound to dcraw_clear_mem. Its bmp branch and the black-level loop's cblack[4]*cblack[5] indexing also moved off 32-bit arithmetic. Added four small DNG fuzz fixtures, built by a committed generator. Wired testsuite/raw/src into the corpus sources; raw had no seeds before, since real raw files exceed the per-file size cap. The flip regression compares each code against the unflipped read transformed with oiiotool, so it does not drift with the LibRaw version. Assisted-by: Claude Code / claude-opus-5 Signed-off-by: Larry Gritz <lg@larrygritz.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
More auditing of the raw plugin turned up additional bugs.
raw_bps is floored at 8 bits per sample. Some headers leave it 0, and Phase One stores a format code there instead. Either value collapsed declared_bytes to 0 and let the bomb guard evaporate.
read_native_scanline's undebayered path (raw:Demosaic=none) mapped only 4 of LibRaw's 8 flip codes. Flip 1, 2, 4 and 7 fell through the if/else chain and returned true without writing the caller's buffer. That is an uninitialized-heap disclosure, not just wrong data. Flip 3 mirrored only vertically, disagreeing with the debayered path. The chain is replaced by the bitwise mapping flip_index() uses: bit 2 transposes, bit 1 mirrors rows, bit 0 mirrors columns. Flips 0, 5 and 6 are unchanged.
The same path indexed rawdata.raw_image from header-derived extents alone. Offsets were computed in 32-bit int. The index is now computed in 64-bit and bounded against raw_width*raw_height, LibRaw's actual allocation. That bound uses the allocation, not the header geometry, because Fuji rotated-sensor images are wider than the sensor grid.
The debayered path now checks LibRaw's returned channel count and scanline offset against the caller's spec and m_image->data_size before writing.
do_unpack()'s return value was discarded at the call site. m_unpacked was set even when a post-unpack re-open failed. Both are now checked.
exif_parser_cb multiplied tiff_data_size() by an unvalidated count and passed the product to std::vector. tiff_data_size() returns size_t(-1) for an unrecognized type, so the multiply could overflow and throw an unguarded std::length_error. Type, count and read return value are now validated.
get_thumbnail leaked the libraw_processed_image_t from dcraw_make_mem_thumb on every path. It is now owned by a unique_ptr bound to dcraw_clear_mem. Its bmp branch and the black-level loop's cblack[4]*cblack[5] indexing also moved off 32-bit arithmetic.
Added four small DNG fuzz fixtures, built by a committed generator. Wired testsuite/raw/src into the corpus sources; raw had no seeds before, since real raw files exceed the per-file size cap. The flip regression compares each code against the unflipped read transformed with oiiotool, so it does not drift with the LibRaw version.
Assisted-by: Claude Code / claude-opus-5