jpegrutils: fix size_t underflow in getExifAppleHeadroom#401
Open
yusmer96-maker wants to merge 1 commit into
Open
jpegrutils: fix size_t underflow in getExifAppleHeadroom#401yusmer96-maker wants to merge 1 commit into
yusmer96-maker wants to merge 1 commit into
Conversation
In getExifAppleHeadroom(), the computation:
size_t tmpOffset =
tiffHeaderOffset + offsetToIfd - appleMakerNotesHeaderSize + (size_t)tagData;
can underflow when (tiffHeaderOffset + offsetToIfd) is smaller than
appleMakerNotesHeaderSize (14). All operands are attacker-controlled via the
EXIF block of a crafted UltraHDR JPEG:
- tiffHeaderOffset is 6 (the "Exif\0\0" prefix length)
- offsetToIfd is set from tag 0x8769 tagData in a prior IFD
- appleMakerNotesHeaderSize is the constant 14
- tagData is read from the tag entry
With tiffHeaderOffset=6, offsetToIfd=7, tagData=0, the unsigned arithmetic
wraps to SIZE_MAX. The subsequent readU32/readS32 bounds check
(*offset + 4 > size) then also overflows (SIZE_MAX + 4 == 3 on 64-bit),
silently passing the guard and resulting in a 1-byte heap out-of-bounds
read at data[SIZE_MAX].
This patch adds two guards:
1. Before the subtraction, ensure tiffHeaderOffset + offsetToIfd is not
smaller than appleMakerNotesHeaderSize.
2. Before adding tagData, ensure the addition will not overflow.
Both conditions return false (the function's existing error-handling pattern)
when the EXIF data is malformed.
Confirmed via AddressSanitizer with a crafted UltraHDR JPEG decoded through
the public uhdr_decode() API:
heap-buffer-overflow READ of size 1 in ultrahdr::readU32
at jpegrutils.cpp:494
called from ultrahdr::getExifAppleHeadroom at jpegrutils.cpp:593
After the patch, the same input is rejected cleanly and the OOB read no
longer occurs.
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.
In getExifAppleHeadroom(), the computation:
can underflow when (tiffHeaderOffset + offsetToIfd) is smaller than appleMakerNotesHeaderSize (14). All operands are attacker-controlled via the EXIF block of a crafted UltraHDR JPEG:
With tiffHeaderOffset=6, offsetToIfd=7, tagData=0, the unsigned arithmetic wraps to SIZE_MAX. The subsequent readU32/readS32 bounds check (*offset + 4 > size) then also overflows (SIZE_MAX + 4 == 3 on 64-bit), silently passing the guard and resulting in a 1-byte heap out-of-bounds read at data[SIZE_MAX].
This patch adds two guards:
Both conditions return false (the function's existing error-handling pattern) when the EXIF data is malformed.
Confirmed via AddressSanitizer with a crafted UltraHDR JPEG decoded through the public uhdr_decode() API:
heap-buffer-overflow READ of size 1 in ultrahdr::readU32
at jpegrutils.cpp:494
called from ultrahdr::getExifAppleHeadroom at jpegrutils.cpp:593
After the patch, the same input is rejected cleanly and the OOB read no longer occurs.