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

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