Skip to content

jpegrutils: fix size_t underflow in getExifAppleHeadroom#401

Open
yusmer96-maker wants to merge 1 commit into
google:mainfrom
yusmer96-maker:fix/exif-apple-headroom-size-t-underflow
Open

jpegrutils: fix size_t underflow in getExifAppleHeadroom#401
yusmer96-maker wants to merge 1 commit into
google:mainfrom
yusmer96-maker:fix/exif-apple-headroom-size-t-underflow

Conversation

@yusmer96-maker

Copy link
Copy Markdown

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.

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.
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.

1 participant