1583399 - invalid shift in media/libogg/src/ogg_framing.c:63

Status: NEW

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

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.ogg

This is triggered with an UBSan build. To enable this check add the following to your mozconfig:

ac_add_options --enable-address-sanitizer
ac_add_options --enable-undefined-sanitizer="shift"
ac_add_options --disable-jemalloc

media/libogg/src/ogg_framing.c:63:25: runtime error: left shift of 197 by 24 places cannot be represented in type 'int'
    #0 0x7fee1a819d5b in ogg_page_serialno media/libogg/src/ogg_framing.c:63:25
    #1 0x7fee17bd203e in mozilla::OggDemuxer::ReadMetadata() dom/media/ogg/OggDemuxer.cpp:430:20
    #2 0x7fee17bd1ca6 in mozilla::OggDemuxer::Init() dom/media/ogg/OggDemuxer.cpp:173:7
    #3 0x7fee176f4c23 in mozilla::MediaFormatReader::DemuxerProxy::Init()::$_13::operator()() const dom/media/MediaFormatReader.cpp:722:47
    #4 0x7fee176f47e4 in mozilla::detail::ProxyFunctionRunnable<mozilla::MediaFormatReader::DemuxerProxy::Init()::$_13, mozilla::MozPromise<mozilla::MediaResult, mozilla::MediaResult, true> >::Run() objdir-ff-ubsan/dist/include/mozilla/MozPromise.h:1440:29
    #5 0x7fee11694a15 in mozilla::TaskQueue::Runner::Run() xpcom/threads/TaskQueue.cpp:199:12
    #6 0x7fee116ccd46 in nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp:246:14
    #7 0x7fee116cd39c in non-virtual thunk to nsThreadPool::Run() xpcom/threads/nsThreadPool.cpp
    #8 0x7fee116c561d in nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1225:14
    #9 0x7fee116c9f46 in NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:486:10
    #10 0x7fee12858113 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:303:20
    #11 0x7fee126ecf24 in MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:290:3
    #12 0x7fee116c072d in nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:458:11
    #13 0x7fee3836bdd8 in _pt_root nsprpub/pr/src/pthreads/ptthread.c:198:5
    #14 0x7fee37fc06da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #15 0x7fee36f9e88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Comment 1

[(Away)]

Assuming the numbers are valid in that serial, it seems like the only thing needed is to change the type of ogg_page_serialno from int to uint32_t.

Then the callers would need to be fixed up accordingly, though the plus side is a bunch of places would be able to stop casting to uint32_t: https://searchfox.org/mozilla-central/search?q=symbol:ogg_page_serialno&redirect=false

Comment 2

[Thomas Daede [:TD-Linux]]

The best fix for this is just for the libogg function to cast to unsigned when doing the endianness byteswap, then cast back. I can release a new libogg with this change implemented.