GHSA-g7cr-3p24-7wf7: Buffer Overflow

============================================================================ 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 jpegwritemarker() 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 emitbyte() stores one byte past the end and then decrements freeinbuffer from 0. The sizet 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", iccprofile=im.info.get("iccprofile")). 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 jpegwritemarker 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 jpegwritemarker() 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, jpegwritemarker() is entered with freeinbuffer == 0. libjpeg's emitbyte() stores the byte first and tests --freeinbuffer == 0 afterwards, so the sizet 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> "ICCPROFILE\0" 01 01 <65498 bytes> -> im.info["iccprofile"] 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

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

kills the Python process on the stock build at this commit: glibc aborts with realloc(): invalid next size / corrupted size vs. prevsize, or the process takes SIGSEGV inside jpegwritemarker/jpegwritescanlines. Stopping at JpegEncode.c:330 in gdb shows nextoutputbyte - buf == 65536, freeinbuffer == 0, commentsize == 65533 before the call, and nextoutputbyte - buf == 131073, freeinbuffer == 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 jpegwritemarker 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:

python 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 iccprofile block. bufsize = max(len(exif) + 5, len(extra) + 1)

ImageFile.save() then takes max(MAXBLOCK, bufsize, width4) (src/PIL/ImageFile.py:662, MAXBLOCK = 65536), and encode() allocates exactly that: buf = PyBytesFromStringAndSize(NULL, bufsize) (src/encode.c:134; the real-file path encodetofile() 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 wh (or 2wh), 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 jpegstartcompress(&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:

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

case 4: if (context->comment) { jpegwritemarker(&context->cinfo, JPEGCOM, (unsigned char )context->comment, context->commentsize); // :330 — no space check } state->state++;

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

Every other emitting state is guarded — state 2 requires rawExifLen + 5 <= freeinbuffer (:287), state 5 requires 1024 free bytes (:340), state 6 requires 100 (:368). State 4 has no guard at all. The unguarded jpegwritemarker() call was introduced in e50ae85ea406 ("Use jpegwritemarker 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 (freeinbuffer == 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 (emptyoutputbuffer() returns FALSE, JpegEncode.c:45-48), which is why a nearly full buffer is harmless: with 1..N free bytes the comment write decrements freeinbuffer to 0 part-way, libjpeg raises JERRCANTSUSPEND, and the save fails with OSError: broken data stream. Only the exact-fit case skips that check, because libjpeg-turbo's marker writer is

c / jcmarker.c / (dest->nextoutputbyte)++ = (JOCTET)val; if (--dest->freeinbuffer == 0) { if (!(dest->emptyoutputbuffer)(cinfo)) ERREXIT(cinfo, JERRCANTSUSPEND); }

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 jpegwritescanlines() 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 nextoutputbyte - buf (≈167000) as the byte count and encode() calls PyBytesResize(&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 ICCPROFILE chunks into im.info["iccprofile"] (: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["iccprofile"] 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", iccprofile=…) 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 jpegwritescanlines), 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 PyBytesFromStringAndSize(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, jpegstartcompress() 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 freeinbuffer has wrapped) is call jpegwritescanlines(), 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 jpegwritescanlines+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->processdata) 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 (iccprofile=im.info.get("iccprofile") — 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 jpegstartcompress() allocates directly behind it — libjpeg's PERMANENT and IMAGE pools, including the method pointers jpegwritescanlines() 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 grayscaleconvert() copies a chosen 384-byte scanline over PyRuntime, setting allocators.objarena.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(commentsize) 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 ICCPROFILE 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:

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

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 freeinbuffer 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.encodetile 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 jpegwritemarker() it requires room for the 4-byte marker header plus the comment plus libjpeg's one-byte slack (emitbyte must never bring freeinbuffer to 0 with a non-suspending destination), and otherwise returns to the caller for a fresh buffer:

diff --- 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->commentsize + 5 > + context->destination.pub.freeinbuffer) { + break; + } jpegwritemarker(

The encoder then returns with the buffer full, encodetile() 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 encodetile() 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 testiccbigcomment to Tests/testfilejpeg.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/testfilejpeg.py and Tests/testfilempo.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 IMAGINGCODECMEMORY instead of break when the marker does not fit and nothing has been written to the current buffer yet (nextoutputbyte == 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 MAXBYTESINMARKER (65533) with ValueError("Comment is too long") rather than leaving it to libjpeg's JERRBADLENGTH → OSError.

Independently, the "+ 1 spare byte" / "+ 5" reasoning in save() under-counts by the 18–20 bytes jpegstartcompress() 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 comment in JpegImagePlugin.save

Applies to src/libImaging/JpegEncode.c at the commit above.

diff diff --git a/Tests/testfilejpeg.py b/Tests/testfilejpeg.py index b361aef2e..f884883c4 100644 --- a/Tests/testfilejpeg.py +++ b/Tests/testfilejpeg.py @@ -227,6 +227,16 @@ class TestFileJpeg: im1 = self.roundtrip(hopper(), iccprofile=iccprofile) assert im1.info.get("iccprofile") == (iccprofile or None) + def testiccbigcomment(self) -> None: + # The ICC profile may fill the encoder buffer exactly, in which case the + # comment has to be written to the next buffer + comment = b"C" 65533 + for n in range(ImageFile.MAXBLOCK - 40, ImageFile.MAXBLOCK - 30): + iccprofile = b"I" n + im = self.roundtrip(hopper(), iccprofile=iccprofile, comment=comment) + assert im.info["iccprofile"] == iccprofile + assert im.info["comment"] == comment + def testlargeiccmeta(self, tmppath: Path) -> None: # https://github.com/python-pillow/Pillow/issues/148 # Sometimes the meta data on the iccprofile block is bigger than diff --git a/src/PIL/JpegImagePlugin.py b/src/PIL/JpegImagePlugin.py index 4b8d34ca3..1d9f4c2fa 100644 --- a/src/PIL/JpegImagePlugin.py +++ b/src/PIL/JpegImagePlugin.py @@ -805,7 +805,9 @@ def save(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None: ) i += 1 - comment = info.get("comment", im.info.get("comment")) + comment = info.get("comment", im.info.get("comment")) or b"" + if isinstance(comment, str): + comment = comment.encode() # "progressive" is the official name, but older documentation # says "progression" @@ -856,10 +858,13 @@ def save(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None: bufsize += len(exif) + 5 if extra: bufsize += len(extra) + 1 + if comment: + bufsize += len(comment) + 5 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 iccprofile block. - bufsize = max(len(exif) + 5, len(extra) + 1) + # Ensure that our buffer is big enough. Same with the iccprofile block + # and the comment (COM marker). + bufsize = max(len(exif) + 5, len(extra) + 1, len(comment) + 5) ImageFile.save( im, fp, [ImageFile.Tile("jpeg", (0, 0, im.size), 0, rawmode)], bufsize diff --git a/src/libImaging/JpegEncode.c b/src/libImaging/JpegEncode.c index 098e431fc..b6771c6ec 100644 --- a/src/libImaging/JpegEncode.c +++ b/src/libImaging/JpegEncode.c @@ -327,6 +327,11 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8 buf, int bytes) { case 4: if (context->comment) { + // check for comment len + 'COM' header bytes + if (context->commentsize + 5 > + context->destination.pub.freeinbuffer) { + break; + } jpegwritemarker( &context->cinfo, JPEGCOM,

The reproducer

<details> <summary>Reproducer — <code>repro</code>, 3,343 bytes (click to open)</summary>

Its last line is BUG when the issue triggers and OK when it does not. Re-run on a clean build of that commit it printed BUG.

python #!/usr/bin/env python3 Pillow heap buffer overflow in ImagingJpegEncode() (src/libImaging/JpegEncode.c, case 4). Re-saving a JPEG with its own ICC profile, the usual "keep the colours" re-encode: im = Image.open(f); im.save(out, "JPEG", iccprofile=im.info["iccprofile"]) If the ICC segment fills the 65536-byte encoder buffer to the last byte, the COM marker (forwarded from im.info["comment"] automatically) is written with freeinbuffer == 0. libjpeg stores the byte, then decrements the counter, which wraps to ~2^64, and the comment plus the whole compressed image go past the end of the heap buffer. The Python process dies with SIGSEGV or a glibc abort. Runs against the stock build in /src, no rebuild. Takes about 2 seconds. import random import struct import subprocess import sys from io import BytesIO

from PIL import Image

Output-buffer layout at the moment the COM marker is written: ENCODERBUFFER = 65536 # ImageFile.MAXBLOCK; save() never makes it bigger here JFIFHEADER = 20 # SOI + JFIF APP0, written by jpegstartcompress() ICCOVERHEAD = 18 # FFE2 <len> "ICCPROFILE\0" <seq> <count> ICCPAYLOAD = ENCODERBUFFER - JFIFHEADER - ICCOVERHEAD # 65498: exact fit COMMENT = b"C" 65533 # largest legal COM; content and length chosen by the file

What the victim application does with an uploaded file, run in a child process so that we can see it die. RESAVE = """ import sys from io import BytesIO from PIL import Image im = Image.open(BytesIO(sys.stdin.buffer.read())) im.save(BytesIO(), "JPEG", iccprofile=im.info["iccprofile"]) print("save returned normally") """

def segment(marker, payload): return bytes([0xFF, marker]) + struct.pack(">H", len(payload) + 2) + payload

def makejpeg(withcomment): """A valid 256x256 JPEG: an ICC profile of ICCPAYLOAD bytes, optionally a COM.""" noise = random.Random(1234).randbytes(256 256 3) # ~100 KB of image data out = BytesIO() Image.frombytes("RGB", (256, 256), noise).save(out, "JPEG", quality=95) plain = out.getvalue()

icc = segment(0xE2, b"ICCPROFILE\0\x01\x01" + b"I" ICCPAYLOAD) com = segment(0xFE, COMMENT) if withcomment else b"" return plain[:2] + icc + com + plain[2:] # splice in right after SOI

def resave(jpeg): # -X faulthandler: on a fatal signal, print where in Python the child died cmd = [sys.executable, "-X", "faulthandler", "-c", RESAVE] r = subprocess.run(cmd, input=jpeg, captureoutput=True) print((r.stdout + r.stderr).decode(errors="replace").strip()) print(f"-> exit status {r.returncode}") return r.returncode

Control: the same file without a COM segment re-saves fine, so the buffer sizing and the ICC profile on their own are harmless. print("Control: re-save the JPEG without a comment") resave(makejpeg(withcomment=False))

The bug: add a COM segment. The comment is now written into a buffer that has no room left. print(f"\nPoC: re-save the JPEG with a {len(COMMENT)}-byte comment") rc = resave(makejpeg(withcomment=True))

A negative return code means the child was killed by a signal (SIGSEGV or SIGABRT from glibc's heap checks), not by a Python exception. if rc < 0: print(f"\nim.save() killed the process with signal {-rc}: out-of-bounds heap write") print("BUG") else: print("OK") </details>

---