Pillow: Heap buffer overflow writing JPEG comment after ICC profile fills buffer

============================================================================
Heap buffer overflow writing JPEG comment after ICC profile fills buffer
Location: src/libImaging/JpegEncode.c:330  in ImagingJpegEncode()
Commit:   a707c6b21c313880d0771e54965d142dd42358b6
============================================================================

A heap buffer overflow in Pillow's JPEG encoder lets anyone who can supply an image write up to 64 KB of bytes they choose, followed by the rest of the compressed image, past the end of a 64 KB heap buffer. The target is any application that re-saves the image as JPEG while keeping its colour profile. The reliable result is that the Python process is killed outright rather than raising an exception. Because the overwritten memory sits directly in front of function pointers that the encoder calls next, this could allow code execution, though the attached reproducer demonstrates only the crash.

The bug is in ImagingJpegEncode() in src/libImaging/JpegEncode.c. State 4 (line 330) calls jpeg_write_marker() for the COM comment marker without checking that the output buffer has room. Meanwhile, JpegImagePlugin._save() sizes that buffer from the EXIF and ICC/XMP data only and reserves nothing for the comment. When a 65498-byte ICC profile fills the 65536-byte buffer exactly, libjpeg's emit_byte() stores one byte past the end and then decrements free_in_buffer from 0. The size_t counter wraps, so the marker, the comment and the entire compressed image are written out of bounds.

Triggering it takes only a valid JPEG (or PNG) carrying that profile and any comment, passed through Image.open() and then im.save(..., "JPEG", icc_profile=im.info.get("icc_profile")). That is the documented way to preserve colour. The comment is forwarded from im.info automatically, and no non-default setting is involved.

The attached reproducer re-saves such a file on the stock build, and the process dies with SIGSEGV. An identical file without the comment saves normally. An ASan build of Pillow will not flag this, because the stray writes happen inside the system libjpeg-turbo. Valgrind does report it, as an invalid write immediately after the 65569-byte allocation.

The unguarded call was introduced in e50ae85ea406 ("Use jpeg_write_marker to write comment", 2022-12-05) and first shipped in 9.4.0.

Details

ImagingJpegEncode() (src/libImaging/JpegEncode.c) is a resumable state machine that fills a caller-supplied output buffer of bytes bytes and returns when it is full. State 4 writes the JPEG COM marker with jpeg_write_marker() without first checking that the buffer has any room left. When the preceding state 3 — the verbatim copy of the "extra" APP segments (ICC profile / XMP) — ends exactly at the last byte of the buffer, jpeg_write_marker() is entered with free_in_buffer == 0. libjpeg's emit_byte() stores the byte first and tests --free_in_buffer == 0 afterwards, so the size_t counter wraps to 2^64-1 instead of triggering the "buffer full" path, and the 4-byte marker header, the whole comment, and then (state 5) the DQT/SOF/DHT/SOS headers and the entire entropy-coded image are written linearly past the end of the 65536-byte heap buffer allocated in src/encode.c.

A JPEG laid out as

FFD8                                   SOI
FFE2 <len> "ICC_PROFILE\0" 01 01 <65498 bytes>     -> im.info["icc_profile"]
FFFE <len> <comment, any length 1..65533>           -> im.info["comment"]
<any baseline RGB JPEG body>

opened with Image.open() and re-saved with the usual colour-profile-preserving call

im.save(out, "JPEG", icc_profile=im.info.get("icc_profile"))

kills the Python process on the stock build at this commit: glibc aborts with realloc(): invalid next size / corrupted size vs. prev_size, or the process takes SIGSEGV inside jpeg_write_marker/jpeg_write_scanlines. Stopping at JpegEncode.c:330 in gdb shows next_output_byte - buf == 65536, free_in_buffer == 0, comment_size == 65533 before the call, and next_output_byte - buf == 131073, free_in_buffer == 18446744073709486079 after it — 65537 bytes already written beyond a 65536-byte buffer before a single scanline has been encoded. An ASan build of Pillow does not flag this, because the out-of-bounds stores happen inside the (uninstrumented) system libjpeg-turbo; valgrind does, reporting Invalid write ... 0 bytes after a block of size 65,569 from jpeg_write_marker called at JpegEncode.c:330 before aborting on the corrupted heap metadata.

Code path

The buffer. JpegImagePlugin._save() sizes the encoder buffer from the metadata it is about to emit, allowing one spare byte after the extra block and nothing for the comment:

# src/PIL/JpegImagePlugin.py:859-862
    else:
        # The EXIF info needs to be written as one block, + APP1, + one spare byte.
        # Ensure that our buffer is big enough. Same with the icc_profile block.
        bufsize = max(len(exif) + 5, len(extra) + 1)

ImageFile._save() then takes max(MAXBLOCK, bufsize, width*4) (src/PIL/ImageFile.py:662, MAXBLOCK = 65536), and _encode() allocates exactly that: buf = PyBytes_FromStringAndSize(NULL, bufsize) (src/encode.c:134; the real-file path _encode_to_file() does malloc(bufsize) at src/encode.c:190). For the input above len(extra) == 65516, so the buffer is 65536 bytes. The optimize/progressive branch just above (JpegImagePlugin.py:846-858) is not immune: its bufsize adds w*h (or 2*w*h), but the image dimensions and the ICC length both come from the file, so an exact fit is still constructible there (e.g. with a 19×1 image).

The "+1 spare byte" does not account for what libjpeg itself has already put in the buffer: in state 1 jpeg_start_compress(&cinfo, TRUE) (JpegEncode.c:279) emits SOI + JFIF APP0, 20 bytes for an RGB source. State 3 then copies the extra block, clamped to the space remaining:

// src/libImaging/JpegEncode.c:302-337
        case 3:
            if (context->extra) {
                unsigned int n = context->extra_size - context->extra_offset;
                if (n > context->destination.pub.free_in_buffer) {
                    n = context->destination.pub.free_in_buffer;
                }
                memcpy(context->destination.pub.next_output_byte, context->extra + context->extra_offset, n);
                context->destination.pub.next_output_byte += n;
                context->destination.pub.free_in_buffer -= n;
                context->extra_offset += n;
                if (context->extra_offset >= context->extra_size) {
                    state->state++;                 // done -> fall into case 4, even with free_in_buffer == 0
                } else {
                    break;
                }
            } …

        case 4:
            if (context->comment) {
                jpeg_write_marker(&context->cinfo, JPEG_COM,
                                  (unsigned char *)context->comment, context->comment_size);   // :330 — no space check
            }
            state->state++;

        case 5:
            if (1024 > context->destination.pub.free_in_buffer) {   // passes: free_in_buffer is now ~2^64
                break;
            }
            … jpeg_write_scanlines(…) …

Every other emitting state is guarded — state 2 requires rawExifLen + 5 <= free_in_buffer (:287), state 5 requires 1024 free bytes (:340), state 6 requires 100 (:368). State 4 has no guard at all. The unguarded jpeg_write_marker() call was introduced in e50ae85ea406 ("Use jpeg_write_marker to write comment", 2022-12-05) and first shipped in 9.4.0; at 9.3.0 the same input re-saves correctly.

With 20 header bytes plus 65516 extra bytes the copy consumes the buffer to the byte (free_in_buffer == 0) and, because the extra block is complete, control falls straight into state 4. One re-wrapped ICC APP2 segment costs 18 bytes of overhead (`JpegIma