From 10a7885504e49b66a63499fc9d209b5855ae167e Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 5 Oct 2026 12:12:04 +1100 Subject: [PATCH] Check COM marker length when encoding JPEG Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Tests/test_file_jpeg.py | 18 ++++++++++++++++++ src/PIL/JpegImagePlugin.py | 10 ++++++++-- src/libImaging/JpegEncode.c | 5 +++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/Tests/test_file_jpeg.py b/Tests/test_file_jpeg.py index b361aef2eee..799e93fdd26 100644 --- a/Tests/test_file_jpeg.py +++ b/Tests/test_file_jpeg.py @@ -129,6 +129,14 @@ def test_comment_write(self) -> None: with Image.open(out) as reloaded: assert reloaded.info["comment"] == b"Test comment text" + def test_large_comment(self, tmp_path: Path) -> None: + f = tmp_path / "temp.jpg" + im = hopper() + im.save(f, "JPEG", comment=b"1" * 65533) + + with pytest.raises(ValueError, match="Comment is too long"): + im.save(f, "JPEG", comment=b"1" * 65534) + def test_cmyk(self) -> None: # Test CMYK handling. Thanks to Tim and Charlie for test data, # Michael for getting me to look one more time. @@ -227,6 +235,16 @@ def test_icc_big(self, n: int) -> None: im1 = self.roundtrip(hopper(), icc_profile=icc_profile) assert im1.info.get("icc_profile") == (icc_profile or None) + def test_icc_big_comment(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): + icc_profile = b"I" * n + im = self.roundtrip(hopper(), icc_profile=icc_profile, comment=comment) + assert im.info["icc_profile"] == icc_profile + assert im.info["comment"] == comment + def test_large_icc_meta(self, tmp_path: Path) -> None: # https://github.com/python-pillow/Pillow/issues/148 # Sometimes the meta data on the icc_profile block is bigger than diff --git a/src/PIL/JpegImagePlugin.py b/src/PIL/JpegImagePlugin.py index da7a1e39d71..e72293223fc 100644 --- a/src/PIL/JpegImagePlugin.py +++ b/src/PIL/JpegImagePlugin.py @@ -804,6 +804,9 @@ def validate_qtables( i += 1 comment = info.get("comment", im.info.get("comment")) + if comment and len(comment) > MAX_BYTES_IN_MARKER: + msg = "Comment is too long" + raise ValueError(msg) # "progressive" is the official name, but older documentation # says "progression" @@ -854,11 +857,14 @@ def validate_qtables( bufsize += len(exif) + 5 if extra: bufsize += len(extra) + if comment: + bufsize += len(comment) + 5 else: # The EXIF info needs to be written as one block, + APP1, + one spare byte, # as libjpeg without suspension requires one byte left after writing a marker. - # Ensure that our buffer is big enough. Same with the icc_profile block. - bufsize = max(len(exif) + 5, len(extra)) + # Ensure that our buffer is big enough. Same with the icc_profile block + # and the comment (COM marker). + bufsize = max(len(exif) + 5, len(extra), len(comment or b"") + 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 098e431fca0..b6771c6ec09 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->comment_size + 5 > + context->destination.pub.free_in_buffer) { + break; + } jpeg_write_marker( &context->cinfo, JPEG_COM,