2019062 - Integer overflow issue in NativeLayerRootRemoteMacParent::RecvRequestReadback (testing-only)

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Andrew McCreight [:mccr8]]

An AI audit found this issue, but the actual report is kind of bogus.

Anyways, NativeLayerRootRemoteMacParent::RecvRequestReadback has this code:

size_t readbackSize = aSize.width * aSize.height * gfx::BytesPerPixel(readbackFormat);

aSize is IntSize, and there's no validation of it, so I think width and height are int32. They could be negative or overflow which I could believe can cause problems later.

However, this isn't actually a security issue because this method fails if !xpc::IsInAutomation() so an attacker can't get at this code.

Comment 1

[Brad Werth [:bradwerth]]

I will build a fix.

Comment 2

[Brad Werth [:bradwerth]]

Attachment: (secure)

Comment 3

[Brad Werth [:bradwerth]]

Comment on attachment 9547677 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Seemingly not possible. The function has an early exit for non-automation calls. Otherwise, the intent of the size validation is clear and an oversized buffer could be created.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: easy to create
• How likely is this patch to cause regressions; how much testing does it need?: unlikely to cause regression; would require a wildly high resolution that won't occur in the wild.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: No

Comment 4

[Andrew McCreight [:mccr8]]

This is sec-other so you don't need sec-approval to land it.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4a51574dd784
https://hg.mozilla.org/integration/autoland/rev/ac80645d490f
Make NativeLayerRootRemoteMacParent::RecvRequestReadback reject invalid sizes. r=gfx-reviewers,lsalzman

Comment 6

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

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