1649862 - load of value 128, which is not a valid value for type 'enum Dav1dMatrixCoefficients' in dom/media/platforms/agnostic/DAV1DDecoder.cpp:188

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.mp4

Found with m-c 20200630-47f18d1138df

/gecko/dom/media/platforms/agnostic/DAV1DDecoder.cpp:188:31: runtime error: load of value 128, which is not a valid value for type 'enum Dav1dMatrixCoefficients'
    #0 0x7f5580a7cb7d in mozilla::DAV1DDecoder::ConstructImage(Dav1dPicture const&) /gecko/dom/media/platforms/agnostic/DAV1DDecoder.cpp:188:31
    #1 0x7f5580a7ad57 in mozilla::DAV1DDecoder::GetPicture(nsTArray<RefPtr<mozilla::MediaData> >&, mozilla::MediaResult&) /gecko/dom/media/platforms/agnostic/DAV1DDecoder.cpp:164:25
    #2 0x7f5580a796d6 in mozilla::DAV1DDecoder::InvokeDecode(mozilla::MediaRawData*) /gecko/dom/media/platforms/agnostic/DAV1DDecoder.cpp:125:11
    #3 0x7f5580ad6563 in applyImpl<mozilla::DAV1DDecoder, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::DAV1DDecoder::*)(mozilla::MediaRawData *), StoreRefPtrPassByPtr<mozilla::MediaRawData> , 0> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1188:12
    #4 0x7f5580ad6563 in apply<mozilla::DAV1DDecoder, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::DAV1DDecoder::*)(mozilla::MediaRawData *)> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1194:12
    #5 0x7f5580ad6563 in mozilla::detail::MethodCall<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::DAV1DDecoder::*)(mozilla::MediaRawData*), mozilla::DAV1DDecoder, mozilla::MediaRawData*>::Invoke() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:1437:47
    #6 0x7f5580ad5d63 in mozilla::detail::ProxyRunnable<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>, RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > (mozilla::DAV1DDecoder::*)(mozilla::MediaRawData*), mozilla::DAV1DDecoder, mozilla::MediaRawData*>::Run() /builds/worker/workspace/obj-build/dist/include/mozilla/MozPromise.h:1457:42
    #7 0x7f557982c1a1 in mozilla::TaskQueue::Runner::Run() /gecko/xpcom/threads/TaskQueue.cpp:158:20
    #8 0x7f557985c536 in nsThreadPool::Run() /gecko/xpcom/threads/nsThreadPool.cpp:299:14
    #9 0x7f557984d345 in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1234:14
    #10 0x7f557985823c in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #11 0x7f557abf1602 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:302:20
    #12 0x7f557aacc8c7 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #13 0x7f557aacc8c7 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #14 0x7f557aacc8c7 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #15 0x7f5579845d07 in nsThread::ThreadFunc(void*) /gecko/xpcom/threads/nsThread.cpp:447:10
    #16 0x7f559eca1d3e in _pt_root /gecko/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #17 0x7f559e8e26da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #18 0x7f559d8c088e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[Alex Chronopoulos [:achronop]]

How did you test, I loaded the file in a local build and I got a different crash:

Assertion failure: aTime >= 0.0 (Cannot seek to a negative value.), at mozilla/firefox/dom/media/MediaDecoder.cpp:614

Comment 2

[Tyson Smith [:tsmith]]

(In reply to Alex Chronopoulos [:achronop] from comment #1)

How did you test

This is caught by the UBSan check[1] enum. TC ASan builds have this enabled by default. You can also add ac_add_options --enable-undefined-sanitizer="enum" to your mozconfig.

[1] https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#id4

Comment 3

[Jon Bauman [:jbauman:]]

I'll take a look at this

Comment 4

[Jon Bauman [:jbauman:]]

Even with the negative time assertions disabled and adding ac_add_options --enable-undefined-sanitizer="enum", I'm not able to reproduce this. In fact, I'm not seeing it even get to DAV1DDecoder::Init (though it does make it through MP4Metadata::Parse).

When I trace through execution I see we make it to AppleVTDecoder::Decode eventually, so it doesn't look like the code even thinks this is AV1 encoded. Which, now that I look at the file more, I see it isn't. Looking at the samples, it appears to be AVC1, so I'm at a loss for why we're calling into DAV1DDecoder code at all. Perhaps is a platform-specific difference, but how exactly was this file loaded in the test case?

Comment 5

[Tyson Smith [:tsmith]]

Attachment: testcase.webm

Not sure what's up. Maybe I attached a different test case? If so sorry about that.

This one works. I'll create a Pernosco session too.

Comment 6

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/fGg14KQHnlK-pDE4pBd3JQ/index.html

Comment 7

[Jon Bauman [:jbauman:]]

Ah, this makes much more sense, thank you.

So, I'm able to reproduce this now, but I'm a little bit at a loss for what we should do to address it. I see from this link that an invalid enumeration value (for which 128 certainly is, given that Dav1dMatrixCoefficients is only defined for 0–14), but we have no way of checking for that (short of an incredibly ugly cast and check) without running afoul of UB, so I think the checking would have to occur in dav1d itself and return an error.

I'm not the security/fuzzing expert, so let me know if that sounds right to you and I can take it up with the dav1d developers.

Comment 8

[Jon Bauman [:jbauman:]]

I've discussed this with the dav1d developers and now have an issue to track its resolution.

Comment 9

[Jon Bauman [:jbauman:]]

https://code.videolan.org/videolan/dav1d/-/merge_requests/1051 has a fix that I've successfully tested locally. Assuming that merges soon, this bug should be closed upon our next dav1d update, which I've added as a blocking bug.

Comment 10

[Jon Bauman [:jbauman:]]

The dav1d fix has been merged, so this is no longer stalled. Unless there's a compelling reason to do it earlier, this bug will be fixed along with our next scheduled dav1d update at the end of July.

Comment 11

[Jon Bauman [:jbauman:]]

This should be fixed now that bug 1652202 brought in [the fixed code] from dav1d into our tree.