1471148 - Hit MOZ_CRASH(media/libopus/celt/celt_decoder.c:125 assertion failed: st->start < st->end) at nil:16 ([@ celt_fatal] through [@ celt_decode_with_ec])

Status: RESOLVED WORKSFORME

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

Description

[Christian Holler (:decoder)]

Using jesup's patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1447133#c11 on top of mozilla-central revision 6e8e861540e6 I was able to find one of the fatal aborts added by that patch.

Testcase and crash information are attached.

However, the testcase comes from the media fuzzing target in mozilla-central, which hasn't landed yet. It is a direct input to the OggDemuxer running in the Benchmark class. If the attachments alone aren't enough to figure out what's going on, I can provide a patch to reproduce this locally. Marking s-s because the original bug is s-s.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Jean-Marc Valin]

These assertions are designed to trigger if the Opus state got corrupted by the application calling it. That being said, I've had a false positive (harmless case) with that particular assertion in the past, so it would be good to check that first. Are you able to extract the value of st->start and st->end? Also, can you attach just the Opus file you're sending to the demuxer?

Comment 4

[Christian Holler (:decoder)]

One of the tests is attached, this is what gets fed to the OggDemuxer. There is no way for me to extract anything below that level.

For the attached file, st->start and st->end are identical, both are the value 17. I also have another testcase where st->start is 17 and st->end is 13.

Comment 5

[Jean-Marc Valin]

OK, this looks a lot like the false positive I was mentioning. It's something a bit strange the decoder does, but that doesn't result in any bad behaviour. I've still fixed it in this commit: https://git.xiph.org/?p=opus.git;a=commitdiff;h=92ffce621df
Can you check if that commit is part of the patchset you're testing? If not, it may be best to test with Opus master instead. If the patch is part of what you've been testing, then I guess it means I haven't fixed all cases, of that weird behaviour.

In the mean time, if you want to continue fuzzing the same code without that false positive, you can simply remove the celt_assert(st->start < st->end); and keep the others.

Comment 6

[Christian Holler (:decoder)]

I confirmed that another patch from Jean-Marc fixed this bogus assert. Since none of this code is in our codebase yet, closing this as WFM.