Closed Bug 1532861 Opened 9 months ago Closed Last month

UBSan: signed integer overflow in [@ mozilla::IsValidVideoRegion]

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr68 --- wontfix
firefox67 --- wontfix
firefox71 --- fixed

People

(Reporter: tsmith, Assigned: jya)

References

(Blocks 2 open bugs)

Details

(Keywords: csectype-undefined, testcase)

Attachments

(2 files, 1 obsolete file)

Found in m-c commit 78601cacfe69

This was build with undefined behavior sanitizer checks enabled via mozconfig.
ac_add_options --enable-undefined-sanitizer="signed-integer-overflow"

src/dom/media/VideoUtils.cpp:201:25: runtime error: signed integer overflow: -2082471576 * 640 cannot be represented in type 'int'
    #0 0x7f272a626e46 in mozilla::IsValidVideoRegion(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) src/dom/media/VideoUtils.cpp:201:25
    #1 0x7f272ae0d9d7 in mozilla::WebMDemuxer::ReadMetadata() src/dom/media/webm/WebMDemuxer.cpp:328:12
    #2 0x7f272ae0c80f in mozilla::WebMDemuxer::Init() src/dom/media/webm/WebMDemuxer.cpp:181:7
    #3 0x7f272a7e9bb3 in operator() src/dom/media/MediaFormatReader.cpp:898:47
    #4 0x7f272a7e9bb3 in mozilla::detail::ProxyFunctionRunnable<mozilla::MediaFormatReader::DemuxerProxy::Init()::$_9, mozilla::MozPromise<mozilla::MediaResult, mozilla::MediaResult, true> >::Run() src/objdir-ff-ubsan/dist/include/mozilla/MozPromise.h:1419
    #5 0x7f272517f450 in mozilla::TaskQueue::Runner::Run() src/xpcom/threads/TaskQueue.cpp:199:12
    #6 0x7f27251aefc3 in nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp:241:14
    #7 0x7f27251af33c in non-virtual thunk to nsThreadPool::Run() src/xpcom/threads/nsThreadPool.cpp
    #8 0x7f27251a6fd1 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1166:14
    #9 0x7f27251ab9fd in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:482:10
    #10 0x7f27262a9c12 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:333:5
    #11 0x7f2726170650 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308:3
    #12 0x7f2726170650 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #13 0x7f27251a1902 in nsThread::ThreadFunc(void*) src/xpcom/threads/nsThread.cpp:453:11
    #14 0x7f274b42f592 in _pt_root src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #15 0x7f274b0af6da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #16 0x7f274a08d88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Attached video testcase.webm
Keywords: testcase
Rank: 15
Priority: -- → P2
Assignee: nobody → jyavenard

It looks like this patch landed a long time ago but that information didn't propagate to bugzilla. Could you check that we no longer hit this issue?

Flags: needinfo?(twsmith)

I am able to reproduce the issue with the attached test case.

Ah, I see the problem. That patch was for bug 1533127 but got opened against this one. It was eventually resolved against the right bug though.

The variables in this function are really unsigned values in disguise. Ideally they'd be something like UIntSize but that's not a thing. As a path of least resistance, let's check that they're greater than zero to rule out absurdly large unsigned values (which would have been ruled out by the MAX_DIMENSION test anyway). And then we no longer need the a*b != 0 tests.

Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7470ae6e250d
Avoid a ubsan error in IsValidVideoRegion r=jya
Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.