2020538 - animated avif (isobmff) mp4 sample offset narrowing can produce a negative read offset, leading to an out-of-bounds read in the avif decode stream

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Oleh Konko [:1seal]]

Attachment: poc.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36

Steps to reproduce:

poc.zip - run the included asan/ubsan harness:

• unzip -q -o poc.zip -d poc && cd poc
• make canonical (demonstrates the u64→i64 narrowing into a negative offset and triggers an asan read)
• make control (validates offset and does not trigger a negative read)

Actual results:

• offsets greater than INT64_MAX can narrow from uint64_t into a negative int64_t.
• downstream logic can end up calling a stream ReadAt() with a negative offset; the decoder uses that value in pointer arithmetic/copy, producing an out-of-bounds read (deterministic under asan in the harness).

Expected results:

• offsets should be range-checked before any signed conversion (reject offset > INT64_MAX and validate offset + size overflow).
• ReadAt()/stream reads should enforce offset >= 0 and offset + size <= length before pointer arithmetic.

===

ADDNEDUM

affected code (pinned)

• mp4 index / byte range construction: https://github.com/mozilla/gecko-dev/blob/5836a062726f715fda621338a17b51aff30d0a8c/dom/media/mp4/SampleIterator.cpp#L475
• avif decode stream read: https://github.com/mozilla/gecko-dev/blob/5836a062726f715fda621338a17b51aff30d0a8c/image/decoders/nsAVIFDecoder.cpp#L140

root cause

mp4parse offsets are represented as u64. when these offsets are stored or compared using signed ranges (int64_t), values greater than INT64_MAX can narrow into negative values. downstream logic that only checks end > length can be bypassed by a negative end, and the resulting negative ReadAt(offset, ...) can reach buffer pointer arithmetic without an offset >= 0 check.

reproduction (attached: poc.zip)

canonical:

unzip poc.zip -d poc
cd poc
make canonical
cat canonical.log

expected canonical output includes markers and an asan crash:

[CALLSITE_HIT]: dom/media/mp4/SampleIterator.cpp:475
[PROOF_MARKER]: u64 offset narrowed into negative int64_t and used for buffer read

control:

unzip poc.zip -d poc
cd poc
make control
cat control.log

expected control output:

[NC_MARKER]: offset validated; no negative offset reaches buffer read

recommended fix

• reject sample offsets > INT64_MAX (and offset+size overflow) before constructing any int64_t byte ranges
• harden the avif stream ReadAt() implementation to validate 0 <= offset <= length and offset + size <= length before any pointer arithmetic or copy

Comment 1

[Daniel Veditz [:dveditz]]

there is no avif in the zip file. Do you have a testcase that demonstrates this out of bounds read is possible in a release Firefox?

Comment 2

[Oleh Konko [:1seal]]

thanks for checking. you’re right: the attached poc.zip is an asan/ubsan harness that models the u64→int64 narrowing and a negative-offset read primitive; it does not contain an actual .avif bitstream.

i can attach small .avif sequence test files that set co64/stco chunk offsets to 0x8000000000001000 (> INT64_MAX). mp4parse (read_avif) accepts them and reports u64 offsets > INT64_MAX, which narrow to negative int64_t when stored in MediaByteRange<int64_t>.

however, i do not yet have a testcase that produces an observable crash/oob read in an uninstrumented release Firefox. with Firefox 145.0.2 (release) and 149.0a1 (nightly), even with image.avif.sequence.enabled=true and image.avif.compliance_strictness=0, the test files are treated as broken/invalid (img.decode(): “Invalid encoded image data”), and i also did not observe an AddressSanitizer report in the macos asan-fuzzing build with the current inputs.

if useful for triage, i can attach:

• avis_co64_int64max.avif (minimal avis+co64)
• valid_seq_co64_int64max_shift.avif (avifenc-produced sequence patched to co64>int64max)
• repro_seq_valid.html (loads the file)
• user.js prefs snippet used for the run

if you’d like, i can also provide a small debug patch that logs/asserts when AVIFDecoderStream::ReadAt() is called with offset < 0, to confirm reachability.

p.s. trying to attach zip, UI is quite complicated

Comment 3

[Oleh Konko [:1seal]]

Attachment: avif_seq_testcases.zip

as requested

Comment 4

[Matthew Gregan [:kinetik]]

There's a bug here worth fixing, thanks for reporting it. I'll post my security analysis later today to aid in triaging this.

Comment 5

[Frederik Braun [:freddy]]

Adding bugmon to attempt automatic verification.

Comment 6

[Matthew Gregan [:kinetik]]

I've got a fix ready for this and a few other places in Gecko's integration with mp4parse's C FFI API that could be more careful about type conversion, I'll post the patch shortly.

I don't believe there's an exploitable security issue here - despite the potential to overflow during uint64_t -> int64_t conversion during sample offset conversion, any invalid values are rejected by all ByteStream::ReadAt implementation (and again in lower layers the ByteStream impls call into) and ultimately result in SampleIterator::GetNext() returning an error. There's no path where an out-of-bounds memory access is possible. Verified via code inspection and probe tests to confirm no ASAN report triggered. The patch I mentioned above hardens the code by rejecting the invalid conversion at the source rather than relying on these checks deeper in the call stack.

Comment 8

[Matthew Gregan [:kinetik]]

Attachment: (secure)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/52a6d1ae2055
https://hg.mozilla.org/integration/autoland/rev/724d98a89a4e
Tighten up uint64_t -> int64_t conversions in mp4parse integration paths.  r=media-playback-reviewers,padenot

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/724d98a89a4e