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 (JpegImagePlugin.py:788-806), hence the 65498-byte profile payload. Pillow's destination manager cannot suspend (empty_output_buffer() returns FALSE, JpegEncode.c:45-48), which is why a nearly full buffer is harmless: with 1..N free bytes the comment write decrements free_in_buffer to 0 part-way, libjpeg raises JERR_CANT_SUSPEND, and the save fails with OSError: broken data stream. Only the exact-fit case skips that check, because libjpeg-turbo's marker writer is

/* jcmarker.c */
  *(dest->next_output_byte)++ = (JOCTET)val;
  if (--dest->free_in_buffer == 0) { if (!(*dest->empty_output_buffer)(cinfo)) ERREXIT(cinfo, JERR_CANT_SUSPEND); }

i.e. it writes buf[65536] first and then wraps the counter. From there nothing limits the stream: the comment (up to 65533 attacker-chosen bytes) is written verbatim, state 5's 1024-byte check passes against the wrapped counter, and jpeg_write_scanlines() appends the full compressed image (≈100 KB for a 256×256 noisy source; proportional to the picture for larger ones). If the process survives that, ImagingJpegEncode() returns next_output_byte - buf (≈167000) as the byte count and _encode() calls _PyBytes_Resize(&buf, 167199) on the 65536-byte object, which is where glibc usually notices the smashed neighbouring chunk.

All three ingredients come from the file. JpegImagePlugin reassembles APP2 ICC_PROFILE chunks into im.info["icc_profile"] (:238-241) and stores the COM payload in im.info["comment"] (:194); _save() forwards the comment on its own — comment = info.get("comment", im.info.get("comment")) (:808) — so the application only has to pass the ICC profile through, which is the standard way to keep colours correct when re-encoding (docs/handbook/image-file-formats.rst:608). Unlike exif and xmp, there is no length check on the comment in _save(). A PNG source works the same way: its iCCP chunk becomes im.info["icc_profile"] and a tEXt chunk keyed comment becomes im.info["comment"] (and, being zlib-compressed, the file is only ~60 KB); that variant was also confirmed to crash. thumbnail() and convert() carry im.info along, so the usual open → thumbnail → convert("RGB") → save("JPEG", icc_profile=…) handler reaches the same state. xmp= and extra= feed the same extra block (33 bytes of overhead for the XMP APP1) and reach the same state. If the application also forwards EXIF, the exact-fit condition becomes len(extra) + len(exif) + 4 == 65516, both lengths again taken from the file. The comment length is irrelevant to triggering: a 17-byte "Created with GIMP" comment crashes the stock build just as reliably (SIGSEGV in jpeg_write_scanlines), it only changes how many of the out-of-bounds bytes are chosen verbatim by the attacker.

What lies behind the buffer

The buffer is PyBytes_FromStringAndSize(NULL, 65536) = malloc(65569), a 0x10030-byte glibc chunk, so buf[65536] is the first byte of the next chunk's header. In the same encode() call, right after that allocation, jpeg_start_compress() mallocs libjpeg's PERMANENT pool (chunk 0xa40) and IMAGE pool (chunk 0x3f00); on a groomed or fresh heap these sit directly behind the buffer. The next thing ImagingJpegEncode() does after the COM write (JpegEncode.c:352; the state-5 room check passes because free_in_buffer has wrapped) is call jpeg_write_scanlines(), which dispatches through the method tables in that IMAGE pool before any allocator call touches the heap. In the SIGSEGV layout observed on the stock build the fault is at jpeg_write_scanlines+133: call *0x8(%rax) with %rax pointing just past the end of the buffer at memory now holding comment bytes — an indirect call through a function pointer (cinfo->main->process_data) read from bytes the file supplied.

Impact

Any service that accepts JPEG uploads and re-encodes them as JPEG while preserving the embedded ICC profile (icc_profile=im.info.get("icc_profile") — thumbnailers, CMS and e-commerce image pipelines, Django/Flask upload handlers) can be made to perform a linear heap overwrite starting immediately after a 65536-byte buffer: 4 marker bytes, then up to 65533 bytes taken verbatim from the file's COM segment, then tens to hundreds of kilobytes of compressed image data. No non-default Pillow setting is involved and the file is an otherwise valid 140–210 KB JPEG that decodes normally. The dependable outcome is termination of the worker process by SIGSEGV or a glibc heap-consistency abort (not a Python exception), i.e. denial of service of everything that process serves.

It does not stop at a crash. Both the length and the content of the first 64 KB written out of bounds are attacker-chosen, the victim buffer is an ordinary malloc chunk, and what jpeg_start_compress() allocates directly behind it — libjpeg's PERMANENT and IMAGE pools, including the method pointers jpeg_write_scanlines() calls through immediately afterwards — is predictable. Against a long-lived stdlib-http.server worker running exactly the handler above (stock /src build, CPython 3.12 / glibc 2.36 / libjpeg62-turbo 2.1.5, ASLR/PIE/NX on, default Pillow settings, worker as root, attacker as uid 65534 given only the service address), this overflow was driven to shell command execution by the worker in two independent ways once library base addresses were known: (a) a JPEG with ~2400 APPn spray segments (heap grooming), the 65498-byte ICC profile and a 3020-byte COM whose bytes re-image the two libjpeg pools so that the real grayscale_convert() copies a chosen 384-byte scanline over _PyRuntime, setting allocators.obj_arena.free = system with ctx = the command — the request returns 200, the worker keeps serving, and the next pymalloc arena release runs the command (14/14 and 15/15 runs on fresh workers); (b) a 28-byte COM that rewrites the size/next of a groomed free tcache chunk sitting right behind the buffer so that Pillow's own malloc(comment_size) in the next JPEG save (src/encode.c:1250) returns _imaging.so's writable .got.plt, the comment replaces free with system, and the encoder's cleanup free(context->comment) at JpegEncode.c:376 runs the command (14/14, deterministic once groomed). The address knowledge came from separate memory-disclosure bugs in Pillow's TIFF paths that return library pointers in output pixels/tags; those are independent defects with their own fixes, but any such disclosure — present or future — is enough, and this overflow is the only memory write either chain needs. Fixing it reduces those leaks to disclosures rather than footholds.

Reproducing

repro is a single Python script. It uses the Pillow already built in /src and needs no rebuild. Run it as ./repro. The last line of output is BUG or OK.

The script builds a valid 256×256 baseline JPEG from deterministic noise, so the compressed image data comes to about 100 KB. Right after SOI it adds an APP2 ICC_PROFILE segment (sequence 1 of 1) with a payload of 65536 - 20 - 18 = 65498 bytes. When Pillow re-encodes the image, libjpeg first writes 20 bytes of SOI and JFIF APP0, and then Pillow copies the re-wrapped ICC segment: 18 bytes of header plus the payload. Together those fill the 65536-byte encoder buffer to the last byte. The constants at the top of the script spell out this arithmetic.

Each file goes through the same application code in a child python3 -X faulthandler process:

im = Image.open(BytesIO(data))
im.save(BytesIO(), "JPEG", icc_profile=im.info["icc_profile"])

1. Control. The file has the ICC profile but no COM segment. The save returns normally with exit status 0, which shows that the exact-fit ICC profile alone is harmless.
2. PoC. This is the same file plus a COM segment of 65533 C bytes. JpegImagePlugin._save() forwards im.info["comment"] automatically. Case 4 of ImagingJpegEncode() writes the COM marker into the full buffer, the free_in_buffer counter wraps, and the comment and the entire compressed image are written past the end of the heap buffer.

Expected output: the control prints save returned normally and -> exit status 0. For the PoC, faulthandler prints Fatal Python error: Segmentation fault (sometimes a glibc message such as realloc(): invalid next size followed by Aborted). The Python stack ends in ImageFile._encode_tile called from im.save, then -> exit status -11 (or -6), and finally BUG. The verdict is BUG only if the PoC child was killed by a signal. A normal return or a Python exception such as OSError gives OK.

Suggested fix

The attached patch guards state 4 the way state 2 guards the EXIF marker: before calling jpeg_write_marker() it requires room for the 4-byte marker header plus the comment plus libjpeg's one-byte slack (emit_byte must never bring free_in_buffer to 0 with a non-suspending destination), and otherwise returns to the caller for a fresh buffer:

--- a/src/libImaging/JpegEncode.c
+++ b/src/libImaging/JpegEncode.c
@@ -327,6 +327,11 @@
         case 4:
 
             if (context->comment) {
+                // check for comment len + 'COM' header bytes
+                if (context->comment_size + 5 >
+                    context->destination.pub.free_in_buffer) {
+                    break;
+                }
                 jpeg_write_marker(

The encoder then returns with the buffer full, _encode_tile() calls it again with an empty buffer, and the comment is emitted there. The C hunk is not sufficient on its own: state 4, like state 2, relies on the Python side guaranteeing that an empty buffer is always large enough, and a 65533-byte comment needs 65538 bytes against a 65536-byte MAXBLOCK buffer — with only the C guard the encoder returns 0 bytes and no error forever and _encode_tile() spins. The patch therefore also changes JpegImagePlugin._save() to reserve len(comment) + 5 in both sizing branches (added to bufsize in the optimize/progressive branch, and as a third term of the max() in the plain branch), and normalises the comment to bytes first (str is UTF-8-encoded, None becomes b"") so the length is exact for non-ASCII str comments; encode.c already UTF-8-encodes str via z#, so the bytes written are unchanged. A side effect visible to users: comments of up to 65533 bytes now save successfully in cases that previously raised OSError: broken data stream because the default buffer was too small. The patch adds test_icc_big_comment to Tests/test_file_jpeg.py, which round-trips a 65533-byte comment with ICC profiles sized around the exact-fit boundary; it crashes (or raises OSError) on the unpatched tree and passes with the patch. With the patch applied to a clean tree and rebuilt, the reproducer no longer triggers, and Tests/test_file_jpeg.py and Tests/test_file_mpo.py pass.

The C guard still depends on the Python sizing to avoid a 0-byte/no-error busy loop, exactly as state 2 already does. A more defensive variant would fail with IMAGING_CODEC_MEMORY instead of break when the marker does not fit and nothing has been written to the current buffer yet (next_output_byte == buf), so that a sizing mistake in any caller can never turn into a busy loop. It would also be consistent with exif/xmp handling for _save() to reject comments longer than MAX_BYTES_IN_MARKER (65533) with ValueError("Comment is too long") rather than leaving it to libjpeg's JERR_BAD_LENGTH → OSError.

Independently, the "+ 1 spare byte" / "+ 5" reasoning in _save() under-counts by the 18–20 bytes jpeg_start_compress() writes before the EXIF/extra blocks; that is what lets attacker-sized metadata land the buffer on an exact boundary in the first place. The attached patch does not change that accounting.

The patch

Check free output-buffer space before writing the JPEG COM marker in ImagingJpegEncode, and size the encoder buffer for the com