1181223 - Stagefright: CHECK(data_offset == data_end) failed in SampleTable.cpp

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.mp4

A/SampleTable(34431): /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/media/libstagefright/frameworks/av/media/libstagefright/SampleTable.cpp:604 CHECK(data_offset == data_end) failed.

Comment 1

[Chris Peterson [:cpeterson]]

Gerald, here's another Stagefright bug.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1181223-gtest.patch

Part 1: Added gtest case.

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1181223-handle-wrong-saio-saiz-size.patch

Part 2: Handle wrong saio/saiz size instead of assuming it is correct.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

I should probably have explained why the proposed fix just warns about the issue but continues processing past the oversized 'saio'/'saiz' box.

I initially tried to treat it as an "ERROR_MALFORMED", which would stop processing and reject the file completely. But this made one particular test (test_eme_non_mse_fails.html) fail on almost all platforms:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=29ff724b97e9

My guess is that the test file might have a corrupt (but still usable) saio/saiz box, though the fact that it doesn't fail on Mac (my dev platform of course, making debugging "interesting") is worrying.

Anyway, I decided to just warn&continue, because the box being larger than what its content needs is not a big problem (it shouldn't crash or expose an exploitable vulnerability).
Also libstagefright should become obsolete soon, so trying to fix this properly could be a waste of time.

But we should keep this in mind, in case the new mp4 metadata parser also makes test_eme_non_mse_fails.html fail because of a wrong saio/saiz size.

Ralph, what do you think? Investigate now; open a bug; something else?

Comment 5

[Ralph Giles (:rillian)]

I'd open a new bug if you have time to investigate whether the test file is corrupt, but land this fix in the meantime. It's always good to clear up issues with the test. Or at least document if it's a broken file we need to support.

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 8683543 [details] [diff] [review]
1181223-handle-wrong-saio-saiz-size.patch

Review of attachment 8683543 [details] [diff] [review]:
-----------------------------------------------------------------

I assume the parser recovers by starting the next box at the declared boundary rather than being confused by the partial consumption.

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to Ralph Giles (:rillian) from comment #6)

Thank you for all the reviews today.

> Comment on attachment 8683543 [details] [diff] [review]
> 1181223-handle-wrong-saio-saiz-size.patch
> 
> Review of attachment 8683543 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I assume the parser recovers by starting the next box at the declared
> boundary rather than being confused by the partial consumption.

Correct.

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1181223-gtest.patch

Clarified check-in comment, no actual changes; carrying r+.

Comment 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 1181223-handle-wrong-saio-saiz-size.patch

Added comment to explain that the parser will skip over the oversized box; no actual code changes; carrying r+.

Comment 10

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Check-in in this order: bug 1181719, bug 1181223, bug 1181220, bug 1181215, bug 1181213
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=20e7b755a695

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/741fc69a1679
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd0c64a4656b

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/741fc69a1679
https://hg.mozilla.org/mozilla-central/rev/cd0c64a4656b

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/741fc69a1679
https://hg.mozilla.org/mozilla-central/rev/cd0c64a4656b