stb_image: zero-init paletted-PNG palette buffer#1954
Closed
iliaal wants to merge 1 commit into
Closed
Conversation
`stbi__parse_png_file` declares a 1024-byte palette on the stack but only fills the first `pal_len * 4` entries from the PNG's PLTE chunk. `stbi__expand_png_palette` then dereferences `palette[idx*4..+4]` for every pixel index in [0, 255] without bounds-checking `idx` against `pal_len`, so a paletted PNG referencing indices >= pal_len reads uninitialised stack content into the output raster — a stack info-leak primitive for any caller that emits the decoded image (server-side thumbnailing, screenshot APIs, image-conversion services). Documented as bug nothings#5 in nothings#1928 ("PNG palette index out-of-bounds"); a fix that adds explicit `idx >= len` checks in `stbi__expand_png_palette` is open at nothings#1927. This patch is complementary, not competing: zero-init costs nothing (one bracket-init at declaration, 1024 stack bytes that were already allocated), keeps semantics identical for valid inputs, and makes out-of-range indices render as deterministic black for invalid inputs. The change persists as defence-in-depth even if a future refactor regresses the index check in nothings#1927 — the leak primitive is gone for good. Hit by fastchart (php-ext rasterising user-supplied SVGs via plutosvg → vendored stb_image); shipped as a local patch in fastchart 1.1.1, now offered upstream.
Contributor
|
I think this is a subset of #1863 . |
Author
|
@NBickford-NV right, this is a strict subset of #1863 (same single-char |
Author
|
Closing in favour of #1863 (linked above). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
stbi__parse_png_filedeclarespalette[1024]on the stack. The PLTE chunk fills the firstpal_len*4bytes.stbi__expand_png_palettethen readspalette[idx*4 .. idx*4+4]for every pixel index in[0,255]without bounds-checkingidxagainstpal_len. A paletted PNG referencingidx >= pal_lenreads uninitialized stack into the output raster (return addresses, canaries, prior locals). Bug #5 in #1928.Complements #1927, doesn't compete with it. #1927 adds
idx >= lenchecks instbi__expand_png_paletteand rejects malformed PNGs. This PR zero-inits the buffer instead. With #1927 alone, out-of-range indices error out. With this alone, they render black. With both, the leak is gone even if the index check regresses later. Cost: one C99 brace-init at function entry; the 1024 stack bytes were already there. Matches the existingtc[3]={0}style two lines down.Repro: PoC at
poc/poc_palette_leak.cfrom #1927. Without this patch, 161/255 pixels contain stack bytes. With it, those pixels are zero.Hit by fastchart 1.1.1, a PHP extension that rasterizes user SVG through plutosvg, which vendors stb_image. fastchart 1.1.1 carries this as a local patch; sending upstream so plutosvg and other consumers pick it up.
v2.30, master HEAD. No API change. No allocator paths touched.