1767365 - crash in [@ mozilla::gfx::Premultiply_SSE2]

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20220430-6e268f45bed2 (--enable-debug --enable-fuzzing)

The attached test case only seems to reproduce the issue on debug builds.

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

==18443==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x7f0484cccdf8 (pc 0x7f04901a0968 bp 0x7fff9c79deb0 sp 0x7fff9c79dea8 T18443)
==18443==The signal is caused by a WRITE memory access.
    #0 0x7f04901a0968 in PremultiplyChunk_SSE2<true, true> src/gfx/2d/SwizzleSSE2.cpp:100:5
    #1 0x7f04901a0968 in void mozilla::gfx::Premultiply_SSE2<true, true>(unsigned char const*, int, unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>) src/gfx/2d/SwizzleSSE2.cpp:132:5
    #2 0x7f0490201c7a in mozilla::gfx::PremultiplyData(unsigned char const*, int, mozilla::gfx::SurfaceFormat, unsigned char*, int, mozilla::gfx::SurfaceFormat, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) src/gfx/2d/Swizzle.cpp
    #3 0x7f04921da1ab in mozilla::dom::CanvasRenderingContext2D::PutImageData_explicit(int, int, mozilla::dom::ImageData&, bool, int, int, int, int, mozilla::ErrorResult&) src/dom/canvas/CanvasRenderingContext2D.cpp:5649:3
    #4 0x7f04921da85c in mozilla::dom::CanvasRenderingContext2D::PutImageData(mozilla::dom::ImageData&, int, int, int, int, int, int, mozilla::ErrorResult&) src/dom/canvas/CanvasRenderingContext2D.cpp:5521:3
    #5 0x7f04916447d0 in mozilla::dom::CanvasRenderingContext2D_Binding::putImageData(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/CanvasRenderingContext2DBinding.cpp:4535:28
    #6 0x7f04920eba3c in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3270:13
    #7 0x7f04974bf570 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:420:13
    #8 0x7f04974bed7a in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:507:12
    #9 0x7f04974b6026 in CallFromStack src/js/src/vm/Interpreter.cpp:578:10
    #10 0x7f04974b6026 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3314:16
    #11 0x7f04974ad2b2 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:389:13
    #12 0x7f04974bec76 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:539:13
    #13 0x7f04974c02a8 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:605:8
    #14 0x7f0496199371 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/vm/CallAndConstruct.cpp:117:10
    #15 0x7f0491e0b379 in mozilla::dom::EventListener::HandleEvent(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/EventListenerBinding.cpp:62:8
    #16 0x7f0492646ce6 in void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventListenerBinding.h:65:12
    #17 0x7f0492646a0d in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1310:43
    #18 0x7f04926476dd in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1507:17
    #19 0x7f049263c614 in HandleEvent src/dom/events/EventListenerManager.h:395:5
    #20 0x7f049263c614 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:348:17
    #21 0x7f049263bb62 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:550:16
    #22 0x7f049263e401 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1119:11
    #23 0x7f04941b6343 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1084:7
    #24 0x7f04957de574 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:6458:20
    #25 0x7f04957de025 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:5850:7
    #26 0x7f04957deeaf in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
    #27 0x7f049002b12c in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1377:3
    #28 0x7f049002a66a in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:975:14
    #29 0x7f0490028921 in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) src/uriloader/base/nsDocLoader.cpp:794:9
    #30 0x7f0490029b08 in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:677:5
    #31 0x7f04957ffcfd in nsDocShell::OnStopRequest(nsIRequest*, nsresult) src/docshell/base/nsDocShell.cpp:13855:23
    #32 0x7f048f363130 in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) src/netwerk/base/nsLoadGroup.cpp:614:22
    #33 0x7f048f364663 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:518:10
    #34 0x7f0490ae65ed in mozilla::dom::Document::DoUnblockOnload() src/dom/base/Document.cpp:11658:18
    #35 0x7f0490ab10df in mozilla::dom::Document::UnblockOnload(bool) src/dom/base/Document.cpp:11596:9
    #36 0x7f0490accedb in mozilla::dom::Document::DispatchContentLoadedEvents() src/dom/base/Document.cpp:8131:3
    #37 0x7f0490b7e36b in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1147:12
    #38 0x7f0490b7e36b in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1153:12
    #39 0x7f0490b7e36b in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1200:13
    #40 0x7f048f164082 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:140:20
    #41 0x7f048f192ebe in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:467:16
    #42 0x7f048f16e1d3 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:780:26
    #43 0x7f048f16cd83 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:612:15
    #44 0x7f048f16cff3 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:390:36
    #45 0x7f048f197d06 in operator() src/xpcom/threads/TaskController.cpp:124:37
    #46 0x7f048f197d06 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
    #47 0x7f048f18239f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1180:16
    #48 0x7f048f18878d in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:465:10
    #49 0x7f048fd32486 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:85:21
    #50 0x7f048fc510d7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:380:10
    #51 0x7f048fc50fe2 in RunHandler src/ipc/chromium/src/base/message_loop.cc:373:3
    #52 0x7f048fc50fe2 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:355:3
    #53 0x7f0493df95d8 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #54 0x7f0495f1390b in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:877:20
    #55 0x7f048fd3337a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
    #56 0x7f048fc510d7 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:380:10
    #57 0x7f048fc50fe2 in RunHandler src/ipc/chromium/src/base/message_loop.cc:373:3
    #58 0x7f048fc50fe2 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:355:3
    #59 0x7f0495f12f2c in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:736:34
    #60 0x55bf64af4e30 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #61 0x55bf64af4e30 in main src/browser/app/nsBrowserApp.cpp:327:18
    #62 0x7f04a55700b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
    #63 0x55bf64acabdc in _start (/home/worker/builds/m-c-20220502213947-fuzzing-debug/firefox-bin+0x15bdc) (BuildId: 1b17616686fbacd45e8c11eb3b8c407d45200c2b)

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/aQhjHCzwnAOi0wpkfenAlg/index.html

Note: A -O1 debug build was used because the issue would not reproduce with a no-opt debug build.

Comment 2

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220502213947-a8ce86cf670a.
The bug appears to have been introduced in the following build range:

Start: 740fad344989d083fad880469a0555ac68010d34 (20211208214306)
End: a2943d14c1ab21c30919ef59b056b84a812be565 (20211208190126)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=740fad344989d083fad880469a0555ac68010d34&tochange=a2943d14c1ab21c30919ef59b056b84a812be565

Comment 3

[Daniel Veditz [:dveditz]]

calling this sec-high assuming the worst (that it doesn't really require a debug build to hit this bug, if only we knew the correct incantation)

Comment 4

[Lee Salzman [:lsalzman]]

It looks like this was regressed by bug 1114333 which modified PutImageData in such a way that you can add an almost arbitrary signed offset to a source pointer allowing you to read a lot of data outside of the bounds you should be able to. A similar bug might possibly exist in GetImageData as well, which could let you write outside of bounds.

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: Bug 1767365 - Clip image data transfers. r?aosmond

Comment 6

[Lee Salzman [:lsalzman]]

The patch introduces a new SafeMoveBy method that should ensure that the operation saturates rather than overflows. In addition, I implemented ClipImageDataTransfer so that we avoid offsetting the source rectangle by the negative translation, which would lead to further overflows or out of bounds conditions.

Tyson, can you verify if this patch resolves the issue?

Comment 7

[Tyson Smith [:tsmith]]

Looks good, I am not able to reproduce the issue with the patch applied.

Comment 8

[Lee Salzman [:lsalzman]]

Comment on attachment 9275338 [details]
Bug 1767365 - Clip image data transfers. r?aosmond

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Moderately easily. Knowing that putImageData has a bounds issue is a pretty strong clue to what to do, assuming an attacker knows nothing else about the exploit. It could be used to read data within a 2^31 range of the source image supplied to putImageData. A similar issue might exist with getImageData.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which older supported branches are affected by this flaw?: 91+
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Easy
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely
• Is Android affected?: Yes

Beta/Release Uplift Approval Request

• User impact if declined:
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:
• Is Android affected?: Yes

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Comment 9

[Tom Ritter [:tjr]]

Comment on attachment 9275338 [details]
Bug 1767365 - Clip image data transfers. r?aosmond

Approved to land and request uplift

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Clip image data transfers. r=aosmond
https://hg.mozilla.org/integration/autoland/rev/f43b9c8e0a676e30866163e94ce784ae56f0fc80
https://hg.mozilla.org/mozilla-central/rev/f43b9c8e0a67

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9275338 [details]
Bug 1767365 - Clip image data transfers. r?aosmond

Approved for 101.0b5 and 91.10esr.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4071725e0e0d

Comment 13

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20220510095538-58a6343ab33d.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/e70578598718

Comment 15

[Hani Yacoub, Desktop QA]

Verified as fixed on macOS 11.6, Windows 10 x64 and on Ubuntu 20.04 x64.

Comment 16

[Alex Finder]

Hello! We encountered the following regression for talos on mozilla-beta. Would it be possible for this push to have triggered that regression ?

Comment 17

[Lee Salzman [:lsalzman]]

Offhand, I am not sure how it would cause that regression. But if it was using getImageData, then the improper clipping may have caused it to use a smaller processing area then it otherwise should have been using, giving an artificial speedup that is not correct. If that is the case, I would just accept the regression since it represents the correct performance baseline.

Comment 18

[Lee Salzman [:lsalzman]]

Looking a bit more deeply, none of those tests utilize getImageData or putImageData, so either the regression range is erroneous or one of the other changes in that range is more likely the cause, rather than this bug.