From 466d228135cc645b881118b45aa11c868b192f48 Mon Sep 17 00:00:00 2001 From: Eric Soroos Date: Fri, 2 May 2025 11:50:26 +0200 Subject: [PATCH 1/4] Fix buffer overflow for BcnEncode --- Tests/test_file_dds.py | 15 +++++++++++++++ src/libImaging/BcnEncode.c | 3 +++ 2 files changed, 18 insertions(+) diff --git a/Tests/test_file_dds.py b/Tests/test_file_dds.py index 3388fce164e..12be1056e3b 100644 --- a/Tests/test_file_dds.py +++ b/Tests/test_file_dds.py @@ -511,3 +511,18 @@ def test_save_dx10_bc5(tmp_path: Path) -> None: im = hopper("L") with pytest.raises(OSError, match="only RGB mode can be written as BC5"): im.save(out, pixel_format="BC5") + +@pytest.mark.parametrize( + "pixel_format, mode", + ( + ('DXT1', 'RGBA'), + ('DXT3', 'RGBA'), + ('BC2', 'RGBA'), + ('BC3', 'RGBA'), + ('BC5', 'RGB'), + ), +) +def test_save_large_file(tmp_path: Path, pixel_format: str, mode: str) -> None: + with hopper(mode).resize((440,440)) as im: + # should not error in valgrind + im.save(tmp_path / 'img.dds', 'DDS', pixel_format=pixel_format) diff --git a/src/libImaging/BcnEncode.c b/src/libImaging/BcnEncode.c index 2bad73b9261..f6445d65331 100644 --- a/src/libImaging/BcnEncode.c +++ b/src/libImaging/BcnEncode.c @@ -258,6 +258,9 @@ ImagingBcnEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) { UINT8 *dst = buf; for (;;) { + if (dst + 8 >= bytes + buf) { + break; + } if (n == 5) { encode_bc3_alpha(im, state, dst, 0); dst += 8; From 896b249834c113470bf9355a8c72e2aba9f3fe88 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Fri, 27 Jun 2025 21:19:47 +1000 Subject: [PATCH 2/4] Added additional test case --- Tests/test_file_dds.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Tests/test_file_dds.py b/Tests/test_file_dds.py index 12be1056e3b..8662208007e 100644 --- a/Tests/test_file_dds.py +++ b/Tests/test_file_dds.py @@ -512,17 +512,19 @@ def test_save_dx10_bc5(tmp_path: Path) -> None: with pytest.raises(OSError, match="only RGB mode can be written as BC5"): im.save(out, pixel_format="BC5") + @pytest.mark.parametrize( "pixel_format, mode", ( - ('DXT1', 'RGBA'), - ('DXT3', 'RGBA'), - ('BC2', 'RGBA'), - ('BC3', 'RGBA'), - ('BC5', 'RGB'), + ("DXT1", "RGBA"), + ("DXT3", "RGBA"), + ("DXT5", "RGBA"), + ("BC2", "RGBA"), + ("BC3", "RGBA"), + ("BC5", "RGB"), ), ) def test_save_large_file(tmp_path: Path, pixel_format: str, mode: str) -> None: - with hopper(mode).resize((440,440)) as im: + with hopper(mode).resize((440, 440)) as im: # should not error in valgrind - im.save(tmp_path / 'img.dds', 'DDS', pixel_format=pixel_format) + im.save(tmp_path / "img.dds", "DDS", pixel_format=pixel_format) From 1c5116d43fb33c432f0e542db399804f6ccb06b3 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 7 May 2025 11:53:04 +1000 Subject: [PATCH 3/4] Simplified code --- Tests/test_file_dds.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/test_file_dds.py b/Tests/test_file_dds.py index 8662208007e..5c7a943b1b7 100644 --- a/Tests/test_file_dds.py +++ b/Tests/test_file_dds.py @@ -525,6 +525,6 @@ def test_save_dx10_bc5(tmp_path: Path) -> None: ), ) def test_save_large_file(tmp_path: Path, pixel_format: str, mode: str) -> None: - with hopper(mode).resize((440, 440)) as im: - # should not error in valgrind - im.save(tmp_path / "img.dds", "DDS", pixel_format=pixel_format) + im = hopper(mode).resize((440, 440)) + # should not error in valgrind + im.save(tmp_path / "img.dds", pixel_format=pixel_format) From 8cd86b63626259d70b50545fabb554924ed3bd8a Mon Sep 17 00:00:00 2001 From: Eric Soroos Date: Fri, 2 May 2025 12:56:11 +0200 Subject: [PATCH 4/4] 16 bits per iteration --- src/libImaging/BcnEncode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libImaging/BcnEncode.c b/src/libImaging/BcnEncode.c index f6445d65331..7a5072ddee6 100644 --- a/src/libImaging/BcnEncode.c +++ b/src/libImaging/BcnEncode.c @@ -258,7 +258,8 @@ ImagingBcnEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) { UINT8 *dst = buf; for (;;) { - if (dst + 8 >= bytes + buf) { + // Loop writes a max of 16 bytes per iteration + if (dst + 16 >= bytes + buf) { break; } if (n == 5) {