1846683 - (CVE-2023-5168) Missing array size check in FilterNodeD2D1

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[sonakkbi]

Attachment: result.txt

|FilterNodeD2D1| is used by graphics rendering. In certain situations, |mInputSurfaces| array is accessed in |SetInput()|1. However, the array index, |mIndex| is not verified properly. This is a typical out-of-bounds write scenario and there is an out-of-bounds write in another |SetInput()|2. I tried to make an ASAN report but I found it not easy. Instead, the callstack when crashed is attached.

REPRODUCE
Operating System: Windows
1.apply *patch.diff
2.visit index.html

*: Patch to emulate a compromised content process.

Crash State: see result.txt

Comment 1

[sonakkbi]

Attachment: index.html

Comment 2

[sonakkbi]

Attachment: patch.diff

Comment 3

[Daniel Veditz [:dveditz]]

Looks like we moved the trust boundaries on this code -- it used to be part of the same process.

Comment 4

[Kelsey Gilbert [:jgilbert]]

Can you take a look, Bob?

Comment 5

[Bob Owen (:bobowen)]

Attachment: Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

Comment 6

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

The severity field is not set for this bug.
:bhood, could you have a look please?

For more information, please visit BugBot documentation.

Comment 7

[Bob Owen (:bobowen)]

Comment on attachment 9348869 [details]
Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The issue is obvious, although not necessarily how it can be triggered.
  Also, the example of triggering we have is from another process, so I'm not sure how easy it would be to exploit on its own.
• 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?: all
• 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?:
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, turning debug assertions into release assertions.
• Is Android affected?: No

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 9348869 [details]
Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

sec-approval+ = dveditz

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51b3c33686cf
Make assertions in FilterNodeD2D1::SetInput release assertions. r=jrmuizel

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/51b3c33686cf

Comment 11

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

The patch landed in nightly and beta is affected.
:bobowen, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Bob Owen (:bobowen)]

Comment on attachment 9348869 [details]
Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

Beta/Release Uplift Approval Request

• User impact if declined: Possible use as part of sandbox escape.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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): Just turning debug assertions into release assertions.
• String changes made/needed: None
• Is Android affected?: No

Comment 13

[Bob Owen (:bobowen)]

Comment on attachment 9348869 [details]
Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high
• User impact if declined: Possible use as part of sandbox escape.
• Fix Landed on Version: 119
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just turning debug assertions into release assertions.

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9348869 [details]
Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

Approved for 118.0b6, thanks.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/9c531e4d21ff

Comment 16

[Pascal Chevrel:pascalc]

Comment on attachment 9348869 [details]
Bug 1846683: Make assertions in FilterNodeD2D1::SetInput release assertions. r=jgilbert!

Approved for ESR 115.3, thanks.

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/97b445c58a84

Comment 18

[Tom Ritter [:tjr]]

Attachment: advisory.txt