1989945 - CanvasTranslator: pop_back() on empty vector through [@ mozilla::gfx::DrawTargetWebgl::PopClip]

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Christian Holler (:decoder)]

Apologies for the mangled stack, this is an experimental IPC fuzzing prototype I built to counter some of the recent graphics IPC issues we've seen:

/home/mozilla/.mozbuild/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:1228:2: runtime error: applying non-zero offset 18446744073709551568 to null pointer
    #0 0x7fffd71d72b4 in mozilla::gfx::DrawTargetWebgl::PopClip() (gfx/2d/RecordedEventImpl.h:3029)
    #1 0x7fffd1e97df9 in mozilla::gfx::RecordedPopClip::PlayEvent(mozilla::gfx::Translator*) const
    #2 0x7fffd265fb2a in std::_Function_handler<bool (mozilla::gfx::RecordedEvent*), mozilla::layers::CanvasTranslator::TranslateRecording()::$_1>::_M_invoke(std::_Any_data const&, mozilla::gfx::RecordedEvent*&&)
    #3 0x7fffd1ede58c in std::function<bool (mozilla::gfx::RecordedEvent*)>::operator()(mozilla::gfx::RecordedEvent*) const
    #4 0x7fffd1ea0d37 in bool mozilla::gfx::RecordedEvent::DoWithEvent<mozilla::gfx::MemReader>(mozilla::gfx::MemReader&, mozilla::gfx::RecordedEvent::EventType, std::function<bool (mozilla::gfx::RecordedEvent*)> const&)
    #5 0x7fffd25d5bd3 in mozilla::layers::CanvasTranslator::TranslateRecording()

From the looks, what I think is happening is that we are calling pop_back() here on an empty vector:

https://searchfox.org/firefox-main/rev/c7b4ff2aadfce11e419c226b7839b0348258a504/dom/canvas/DrawTargetWebgl.cpp#1986

This is not safe - calling pop_back() on an empty vector blindly decreases its size and can cause arbitrary memory reads and even double-free if the vector is then used elsewhere (which I believe it can be, because you can issue a series of translation events here).

On Linux, this code runs in the parent, on Windows/Mac it should run in the GPU process, unless that failed enough times to fallback to parent. So this is a potential sandbox escape / privilege escalation.

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: (secure)

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1789849

Comment 3

[Lee Salzman [:lsalzman]]

Comment on attachment 9514778 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not easily
• 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 branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: 106+
• If not all supported branches, which bug introduced the flaw?: Bug 1789849
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely
• Is the patch ready to land after security approval is given?: Yes
• 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: 145
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Beta/Release Uplift Approval Request

• User impact if declined/Reason for urgency:
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• 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

Comment 4

[Tom Ritter [:tjr]]

Comment on attachment 9514778 [details]
(secure)

Approved to land and request uplift

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1e64a0223272
https://hg.mozilla.org/integration/autoland/rev/ed8714d50599
r=ahale

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/ed8714d50599

Comment 8

[Donal Meehan [:dmeehan]]

Comment on attachment 9514778 [details]
(secure)

Rejecting esr128 uplift request, esr128 in now EOL

Comment 9

[Donal Meehan [:dmeehan]]

Comment on attachment 9514778 [details]
(secure)

Approved for 144.0b6

Comment 10

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/016751073374
https://hg.mozilla.org/releases/mozilla-beta/rev/7be4ef988403

Comment 11

[Donal Meehan [:dmeehan]]

Comment on attachment 9514778 [details]
(secure)

Approved for 140.4esr

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/6f6098be40e1

Comment 13

[Ryan VanderMeulen (PTO, back July 27)]

Comment on attachment 9514778 [details]
(secure)

Approved for 115.29esr.

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/b6fc3a535ea5