Skip to content

avif: reject color/alpha picture dimension mismatch instead of silent corruption#3085

Open
scadastrangelove wants to merge 1 commit into
image-rs:mainfrom
scadastrangelove:fix/avif-3084-alpha-dimension-check
Open

avif: reject color/alpha picture dimension mismatch instead of silent corruption#3085
scadastrangelove wants to merge 1 commit into
image-rs:mainfrom
scadastrangelove:fix/avif-3084-alpha-dimension-check

Conversation

@scadastrangelove

Copy link
Copy Markdown

Fixes #3084.

AvifDecoder::read_image (both the 8-bit and 16-bit paths) composited the alpha plane into the output buffer with Iterator::zip, but the primary picture and the alpha picture are decoded independently — each via its own dav1d::Decoder, from its own coded data (AvifDecoder::new). Nothing checked that the two dav1d::Picture results actually had matching width/height before the zip.

Iterator::zip silently stops at the shorter of its two inputs, so a mismatch didn't error — it corrupted the composited image instead.

Verified against current main with a real dav1d-decoded crafted AVIF (color item 8x8, alpha item's own independently-encoded AV1 bitstream 4x4, re-muxed via avif-serialize):

  • Pre-fix: read_image() returned Ok. Rows 0-3 correctly got alpha values from the 4x4 alpha picture, but rows 4-7 got stray alpha bytes not sourced from the visible alpha image at all — dav1d's picture-buffer padding beyond the coded height happened to satisfy the zip's row count. This is silent corruption, not simple truncation.
  • Post-fix: the same file now returns Err("Format error decoding Avif: Alpha plane dimensions (4x4) do not match color plane dimensions (8x8)").

The original issue only flagged the 8-bit path; while fixing it I found the identical unchecked zip in the 16-bit path (process_16bit_picture) and fixed both.

Changes

  • Add a dimension-equality check ahead of both zip loops (8-bit and 16-bit), returning a new AvifDecoderError::AlphaPlaneDimensionMismatch instead of silently compositing.
  • Add a regression test (tests/avif_alpha_dimension_mismatch.rs) that crafts a real, dav1d-decodable dimension-mismatched AVIF at test time (via the crate's own ravif-backed encoder + mp4parse + avif-serialize, no external binary fixture needed) and asserts decoding now errors. Confirmed the test fails on unpatched main and passes with this fix.

Testing

Full suite (cargo test --features avif,avif-native,default-formats,rayon --lib --tests): all green except one pre-existing, unrelated failure in tests/limits.rs::avif (set_limits not enforcing width/height before dav1d decode) — reproduces on unmodified main too, already tracked as #3076.

Discovered by the rust-in-peace security pipeline.

… corruption

AvifDecoder::read_image (both the 8-bit and 16-bit paths) composited the
alpha plane into the output buffer with Iterator::zip, but the primary
picture and the alpha picture are decoded independently -- each via its
own dav1d::Decoder, from its own coded data (AvifDecoder::new). Nothing
checked that the two dav1d::Picture results actually had matching
width/height before the zip.

Iterator::zip silently stops at the shorter of its two inputs, so a
mismatch didn't error, it just corrupted the composited image. Verified
against current main with a real dav1d-decoded crafted AVIF (color item
8x8, alpha item's own AV1 bitstream 4x4, re-muxed via avif-serialize):
pre-fix, read_image() returned Ok with rows 0-3 getting alpha values from
the 4x4 alpha picture and rows 4-7 getting stray alpha bytes not sourced
from the visible alpha image at all (dav1d picture-buffer padding beyond
the coded height). Post-fix, the same file now returns a clear decode
error.

Adds a dimension-equality check ahead of both zip loops, plus a
regression test that crafts a real dimension-mismatched AVIF (via the
crate's own ravif-backed encoder + mp4parse + avif-serialize, no external
fixture needed) and asserts decoding now fails instead of silently
producing a corrupted image.

Addresses image-rs#3084.
Discovered by the rust-in-peace security pipeline
(https://github.com/scadastrangelove/rust-in-peace/).

@197g 197g left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like libavif also has an error code for this, so this is reasonable (with the clippy complaint fixed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AVIF alpha-plane compositing has no dimension-equality check before zip — mismatch silently corrupts alpha instead of erroring

2 participants