Skip to content

Fix heap underflow write in EXR decoder extra-channel framebuffer setup#214

Open
sharadboni wants to merge 1 commit intogoogle:mainfrom
sharadboni:fix/security-exr-decoder-bounds-checks
Open

Fix heap underflow write in EXR decoder extra-channel framebuffer setup#214
sharadboni wants to merge 1 commit intogoogle:mainfrom
sharadboni:fix/security-exr-decoder-bounds-checks

Conversation

@sharadboni
Copy link
Copy Markdown
Contributor

@sharadboni sharadboni commented Apr 28, 2026

Summary

Fix the heap-buffer-underflow write in DecodeImageEXR when a malformed EXR has dataWindow.min.x > 0 (or non-zero start_y in chunked decoding) AND multiple extra channels. This PR was rebased on current main after #216 (the negative-count memcpy fix) was merged, so it now contains only the extra-channel framebuffer fix.

Bug

lib/extras/dec/exr.cc, DecodeImageEXR, line 333 (the extra_rows_ptr initialization):

char* extra_rows_ptr =
    input_extra_rows.data() -
    (dataWindow.min.x + start_y * row_size) * extraPixelBytes;

The offset is computed with extraPixelBytes (sum of all extra channels' per-pixel sizes), but each per-channel OpenEXR::Slice is configured with the per-channel size, size * row_size (line 341). When OpenEXR resolves pixel addresses as extra_rows_ptr + x * size + y * size * row_size, the result underflows input_extra_rows.data() by data_min_x * (extraPixelBytes - size) bytes for channel 0's first pixel (and a similar term for non-zero start_y).

The defect is independent of the negative-count memcpy bug fixed by #216; the windows can overlap normally — only data_min_x > 0 plus multiple extras is required.

Fix

Keep input_extra_rows.data() unmodified as a per-section anchor and compute a separate ch_base_ptr per channel using the channel's own size. With this, OpenEXR's address resolution slice_base + x*size + y*size*row_size lands inside the per-channel section.

Reproduction

PoC config: displayWindow X=[0..99] Y=[0..9], dataWindow X=[50..149] Y=[0..9], 2 HALF extras + RGB HALF, no compression. With data_min_x = 50, extraPixelBytes = 4, per-channel size = 2, channel 0 row 0 underflows by 100 bytes at [data - 100, data - 1].

ASan output (libjxl/jpegli with sanitizers):

AddressSanitizer: attempting free on address which was not malloc()-ed: 0x521000001500
    #0 operator delete(void*, unsigned long)
    #5 std::vector<char>::~vector
    #6 jxl::extras::DecodeImageEXR  lib/extras/dec/exr.cc:413
SUMMARY: AddressSanitizer: bad-free

The std::vector's _M_p was clobbered by the underflow write to a non-malloc'd address; ASan catches the bad-free at scope-exit destruction. Release build (no ASan) trips glibc's heap integrity check (e.g. free(): invalid pointer or double free or corruption (out)) and aborts.

Note on history

This branch was originally filed on Apr 28 with a combined fix for both the line-333 underflow and the line-365 negative-count memcpy. Since then #216 was merged with the line-365 fix in a slightly different shape (using exr_x_span = max(0, exr_x2 - exr_x1 + 1)), so this rebase keeps only the line-333 underflow fix on top of #216.

In DecodeImageEXR, the OpenEXR framebuffer base pointer for extra
channels was computed using extraPixelBytes (sum of all extra
channels' per-pixel sizes), but each per-channel Slice was configured
with the channel's individual size. With dataWindow.min.x > 0 (or
start_y > 0) and multiple extra channels, OpenEXR's pixel address
resolution

    base + x * size + y * size * row_size

underflows the input_extra_rows allocation by
data_min_x * (extraPixelBytes - size) bytes for channel 0's first
pixel, and a similar term for higher chunks. The result is a
heap-buffer-underflow write of attacker-controlled bytes (the EXR
extra-channel pixel data) at a controllable offset before the
allocated buffer.

Fix: keep input_extra_rows.data() unmodified as the per-channel
section base, and compute a separate ch_base_ptr per channel using
that channel's own size. With this, each channel's slice resolves
addresses correctly within its own section.

The negative-count memcpy at exr.cc:365 (also originally addressed
by this branch) was merged separately via google#216 and is unaffected by
this change.
@sharadboni sharadboni force-pushed the fix/security-exr-decoder-bounds-checks branch from 151f7e9 to a41568a Compare May 8, 2026 15:57
@sharadboni sharadboni changed the title Fix heap overflow and underflow in EXR decoder Fix heap underflow write in EXR decoder extra-channel framebuffer setup May 8, 2026
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