1703334 - Crash in [@ mozilla::dom::Element::SetAttr] called from L10nOverlays::OverlayAttributes

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Olli Pettay [:smaug][[e-mail]]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1696575#c3
https://crash-stats.mozilla.org/report/index/6d406d0a-3982-44bc-be96-cdccf0210311
https://crash-stats.mozilla.org/report/index/9000ef46-ec67-4cf2-b1a5-21c5b0210315

Comment 1

[Neha Kochar [:neha]]

This is to fix Andrew's findings in https://bugzilla.mozilla.org/show_bug.cgi?id=1696575#c4 .

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1703334 - nsCOMPtr elem in DOMLocalization::ApplyTranslations().

Comment 3

[Andrew McCreight [:mccr8]]

Comment on attachment 9222659 [details]
Bug 1703334 - nsCOMPtr elem in DOMLocalization::ApplyTranslations().

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I'm not sure, but it is pretty obvious from the patch where you'd start looking.
• 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 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?: No
• If not, how different, hard to create, and risky will they be?: The patch is trivial, so backports should not be an issue.
• How likely is this patch to cause regressions; how much testing does it need?: Very low. It just roots a DOM object on the stack.

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

We're in RC week so we're going to hold this until after we release and put it in 90. I'd rather not land it until midway through 90's cycle.

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9222659 [details]
Bug 1703334 - nsCOMPtr elem in DOMLocalization::ApplyTranslations().

Approved to land and uplift if desired

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 9222659 [details]
Bug 1703334 - nsCOMPtr elem in DOMLocalization::ApplyTranslations().

Beta/Release Uplift Approval Request

• User impact if declined: possible sec issues
• 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): It only roots a variable on the stack.
• String changes made/needed: none

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: 91
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 7

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

nsCOMPtr elem in DOMLocalization::ApplyTranslations(). r=smaug
https://hg.mozilla.org/integration/autoland/rev/1587282e20b3eb403c9e8ffd7f6136f20b0a47aa
https://hg.mozilla.org/mozilla-central/rev/1587282e20b3

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 9222659 [details]
Bug 1703334 - nsCOMPtr elem in DOMLocalization::ApplyTranslations().

approved for 90.0b9 and 78.12esr

Comment 9

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/50f36a0fe297

Comment 10

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/fbc70bf64534

Comment 11

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.