Potential panic in palette.rs / create_rgba_palette

[BenBE]

I have the following PNG file, that is valid according to the PNG specification and has the following overall structure:

+-----------+-----+-----------+----------+
| Offset    | Len | Type      | CRC      |
+-----------+-----+-----------+----------+
| 0x00000000| 8   | SIGNATURE | —        |
| 0x00000008| 13  | IHDR      | 9E2F6E4C |
| 0x00000021| 9   | pHYs      | 38FE363B |
| 0x00000036| 4   | gAMA      | 31E8965F |
| 0x00000046| 3   | sBIT      | C79A0983 |
| 0x00000055| 774 | PLTE      | 3FA76723 |
| 0x00000361| 256 | tRNS      | 2F763A93 |
| 0x00000471| 1   | bKGD      | F545D21B |
| 0x00000480| 31  | IDAT      | 9D74E35B |
| 0x000004B0| 0   | IEND      | AE426082 |
+-----------+-----+-----------+----------+

The file looks like this (base64):

iVBORw0KGgoAAAANSUhEUgAAAAQAAAAECAMAAACeL25MAAAACXBIWXMAAABFAAAARQE4/jY7AAAABGdBTUEAAYagMeiWXwAAAANzQklUBwcHx5oJgwAAAwZQTFRF/v7w
////AAAAAQEBAgICAwMDBAQEBQUFBgYGBwcHCAgICQkJCgoKCwsLDAwMDQ0NDg4ODw8PEBAQEREREhISExMTFBQUFRUVFhYWFxcXGBgYGRkZGhoaGxsbHBwcHR0dHh4e
Hx8fICAgISEhIiIiIyMjJCQkJSUlJiYmJycnKCgoKSkpKioqKysrLCwsLS0tLi4uLy8vMDAwMTExMjIyMzMzNDQ0NTU1NjY2Nzc3ODg4OTk5Ojo6Ozs7PDw8PT09Pj4+
Pz8/QEBAQUFBQkJCQ0NDRERERUVFRkZGR0dHSEhISUlJSkpKS0tLTExMTU1NTk5OT09PUFBQUVFRUlJSU1NTVFRUVVVVVlZWV1dXWFhYWVlZWlpaW1tbXFxcXV1dXl5e
X19fYGBgYWFhYmJiY2NjZGRkZWVlZmZmZ2dnaGhoaWlpampqa2trbGxsbW1tbm5ub29vcHBwcXFxcnJyc3NzdHR0dXV1dnZ2d3d3eHh4eXl5enp6e3t7fHx8fX19fn5+
f39/gICAgYGBgoKCg4ODhISEhYWFhoaGh4eHiIiIiYmJioqKi4uLjIyMjY2Njo6Oj4+PkJCQkZGRkpKSk5OTlJSUlZWVlpaWl5eXmJiYmZmZmpqam5ubnJycnZ2dnp6e
n5+foKCgoaGhoqKio6OjpKSkpaWlpqamp6enqKioqampqqqqq6urrKysra2trq6ur6+vsLCwsbGxsrKys7OztLS0tbW1tra2t7e3uLi4ubm5urq6u7u7vLy8vb29vr6+
v7+/wMDAwcHBwsLCw8PDxMTExcXFxsbGx8fHyMjIycnJysrKy8vLzMzMzc3Nzs7Oz8/P0NDQ0dHR0tLS09PT1NTU1dXV1tbW19fX2NjY2dnZ2tra29vb3Nzc3d3d3t7e
39/f4ODg4eHh4uLi4+Pj5OTk5eXl5ubm5+fn6Ojo6enp6urq6+vr7Ozs7e3t7u7u7+/v8PDw8fHx8vLy8/Pz9PT09fX19vb29/f3+Pj4+fn5+vr6+/v7/Pz8/f39/v7+
////P6dnIwAAAQB0Uk5TAAD/AAAAAAAA/wAAAAAAAAD/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD/AAAAAAAAAAAAAAAAAAAA
AAAAAAD/AAAAAP8AAAAAAAAA/wAAAAAAAAAAAAAAAAAAAAD/AAAAAAAAAAAAAAAAAAAAAAAAAP8AAAAAAAAAAAAAAAAAAAAAAAAAAP8AAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAA/wAAAAD/AAAAAAAAAAAAAAAAAAAAAAAA/wAAAAAAAAAAAAD/AAAAAAAAAAAAAAAAAAAAAAAAAAAA/wAAAAAAAC92OpMAAAABYktHRPVF0hvb
AAAAH0lEQVR4AQEUAOv/AAn5EQIAnXTjWwDYQVbFAGNWiMBFfweaa4QwDwAAAABJRU5ErkJggg==

When parsing this file with image-png, the parser crashes with an OOB write while assigning the parsed palette to the image object in create_rgba_palette (palette.rs).

The reason for this OOB write is a missing length check when writing into the fixed-size rgba_palette array initialized in line 57.

The PNG specification does not limit the maximum length of a palette, but only requires that each color index used in image data is present in the provided palette. Thus an over-long palette is valid according to the PNG specification, although those superfluous entries are useless. When assigning the palette, at most as many entries as fit into the indexable palette should be copied. In 6b1a53d a check was introduced in stream.rs (around line 1040ff) that tries to validate the expected lengths for various chunks and by accident makes the buggy code unreachable.

Furthermore, as a side-note, shorter palettes are valid IF AND ONLY IF no actual pixel data references unassigned palette indices. This constraint is not validated by the parser, thus accepting PNG files that are invalid according to the PNG specification.

Co-Authored-By: @stoeckmann

[fintelia]

Was this report generated by an LLM? I don't think it is right

[BenBE]

When I did the research for this issue, the code of image-png actually caused the OOB write at the specified location (original work was done in October/November). In the meantime since then there was a change introduced as part of #630 (the one regarding missed corrupted CRCs ;-)).

You can test this above PNG file using a commit like f2943fd. Otherwise this should be reproducible with 0.18.0-rc.3, i.e. the latest tag.

The reproducer was mitigated by the change in start_chunk when you introduced the functionality for skipping chunks.

[stoeckmann]

TLDR: I think that the introduced checks are sufficient and this is a non-issue with these checks in place.

Back around October 2025, we investigated the standard compliance of image-png by reading the code base. We stumbled upon 11.2.2 PLTE Palette of the PNG 3 standard.

We focused on:

The number of entries is determined from the chunk length. A chunk length not divisible by 3 is an error.

If this would allow an arbitrarily large palette (since the chunk length could be large), an issue would arise here:

image-png/src/decoder/transform/palette.rs

Lines 64 to 72 in 1764d16

	while palette_iter.len() >= 4 {
	// Copying 4 bytes at a time is more efficient than copying 3.
	// OTOH, this clobbers the alpha value in `rgba_iter[0][3]` - we
	// need to fix this later.
	rgba_iter[0].copy_from_slice(&palette_iter[0..4]);
	palette_iter = &palette_iter[3..];
	rgba_iter = &mut rgba_iter[1..];
	}

You can verify by opening the given file with an image-png parser (before November 2025) and decoder.set_transformations(Transformations::EXPAND) in place. If you put poc.png into top directory of source tree, the simplest and fastest way to reproduce would be a modified change-png-info from examples:

diff --git a/examples/change-png-info.rs b/examples/change-png-info.rs
index 9b332ad..2bf1835 100644
--- a/examples/change-png-info.rs
+++ b/examples/change-png-info.rs
@@ -3,17 +3,18 @@
 use std::fs::File;
 use std::io::{BufReader, BufWriter};
 use std::path::Path;
-
+use png::Transformations;
 use png::DecodingError::LimitsExceeded;
 pub type BoxResult<T> = Result<T, Box<dyn std::error::Error + Send + Sync>>;
 
 fn main() -> BoxResult<()> {
     // # Decode
     // Read test image from pngsuite
-    let path_in = Path::new(r"./tests/pngsuite/basi0g01.png");
+    let path_in = Path::new(r"./poc.png");
     // The decoder is a build for reader and can be used to set various decoding options
     // via `Transformations`. The default output transformation is `Transformations::IDENTITY`.
-    let decoder = png::Decoder::new(BufReader::new(File::open(path_in)?));
+    let mut decoder = png::Decoder::new(BufReader::new(File::open(path_in)?));
+    decoder.set_transformations(Transformations::EXPAND);
     let mut reader = decoder.read_info()?;
     // Allocate the output buffer.
     let png_info = reader.info();

But given the second sentence in the standard

The PLTE chunk contains from 1 to 256 palette entries, each a three-byte series of the form:

I'm reasonably sure that the newly introduced checks are complete and standards compliant. Therefore, the issue should be considered fixed.

[197g]

When parsing this file with image-png, the parser crashes with an OOB write while assigning the parsed palette to the image object in create_rgba_palette

A panic in copy_from_slice is not the same as (a crash due to) an OOB write. Can you confirm which of these it is?

The former aborts deterministically as part of the language semantics before any write to memory occurs while the latter performs a write and merely may result in an access violation at machine semantics. It's denial-of-functionality vs. undefined-behavior/memory corruption/a potential RCE gadget—a vastly different impact metric. Calling the former an OOB write is wrong because the 'write' component is absent. You could try 'attempted OOB write' if you want to obscure the guarantees of memory-safe languages through being technically correct. If it is an OOB write then a RUSTSEC for `0.18´ would be called for.

[BenBE]

It is a panic, which is triggered by the code trying to write data out-of-bounds. Technically that's deterministic, but only in the sense that it's wrong. ;-) If rust were not to enforce the bound checking implicitly (at runtime), this would perform the write. Thus the saving grail is rust doing the OOB check before writing; thus my wording "potential OOB write", not "actual OOB write". I could have written "DoS" too and this would describe what rust makes of this due to its check; doesn't make it better though. Okay, let's move on to being productive.

The issue arises due to a missing size check in create_rgba_palette, which fails to check, that it must copy at most 256 of the entries from the buffer passed in from the PLTE chunk, i.e. that it does not move its write location past the end of the (fixed-size) buffer it allocates for the payload of the PLTE chunk. And while recent versions make it impossible to trigger this bug (due to the mentioned commit), we still have a (naive) loop that copies the amount of data indicated by the (user-controlled) source instead of capping the amount of data to copy at the minimum of input data and target buffer size.

Depending on the reading of the part of the specification quoted above, you can interpret the size of PLTE chunks to be limited to 768 bytes (256 entries á 3 bytes) or that at most 256 of the entries in a PLTE chunk are processed for handling colors. And while I lean towards and agree with the first interpretation, there is IMHO a non-zero chance to interpret the spec according to the second reading, as that one isn't explicitly prohibited (unless I missed some detail).

That said, I'd suggest to make the size check in the loop, that copies the palette over, explicit, such that at most as many entries as are allocated in line 57 are copied over.. And with the range check from PR #630 in place the rust compiler should even be able to see that this (DiD) range check in create_rgba_palette cannot be triggered, and thus optimize it out (unless there are call paths where it cannot).

[fintelia]

I think it would be reasonable to add an assertion to create_rgba_palette to check that the palette has at most 256 entries. But I do agree that this is already fixed by #630.

This is also a good reminder that we should probably make a 0.18.1 release soon.