Bug 1302071 added code which fails the static analysis check in 1336510

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Nika, Assigned: Nika)

Tracking

({sec-high})

unspecified
mozilla54
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox51 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 wontfix, firefox54 fixed)

Details

(Whiteboard: [post-critsmash-triage][adv-main54+])

Attachments

(1 attachment)

I rebased the static analysis onto inbound as I hope to land it as soon as I uplift to beta/aurora, and noticed that a new failure had appeared due to the lambda added here: http://searchfox.org/mozilla-central/rev/4039fb4c5833706f6880763de216974e00ba096c/layout/base/nsPresContext.cpp#3271-3274.

This bug blocks landing the analysis on central.
MozReview-Commit-ID: GRi4afSovP4
Attachment #8842096 - Flags: review?(matt.woodrow)
Assignee: nobody → michael
Attachment #8842096 - Flags: review?(matt.woodrow) → review+
Group: core-security → layout-core-security
Keywords: sec-high
Comment on attachment 8842096 [details] [diff] [review]
Capture a strong reference to nsRootPresContext in EnsureEventualDidPaintEvent

[Security approval request comment]
How easily could an exploit be constructed based on the patch? I imagine it would be quite difficult to exploit this bug.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? The security problem is fairly evident from the commit message and changes made.

Which older supported branches are affected by this flaw? Just nightly, but we're considering uplifting bug 1302071 which would make it affect aurora too.

If not all supported branches, which bug introduced the flaw? bug 1302071

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? There are no other affected branches.

How likely is this patch to cause regressions; how much testing does it need? This is unlikely to cause regressions.
Attachment #8842096 - Flags: sec-approval?
Comment on attachment 8842096 [details] [diff] [review]
Capture a strong reference to nsRootPresContext in EnsureEventualDidPaintEvent

Bugs that only affect trunk can land whenever. Fire away!
Attachment #8842096 - Flags: sec-approval?
https://hg.mozilla.org/mozilla-central/rev/f3ee1078d709
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8842096 [details] [diff] [review]
Capture a strong reference to nsRootPresContext in EnsureEventualDidPaintEvent

This needs to be uplifted if bug 1302071 gets approved.
Attachment #8842096 - Flags: approval-mozilla-aurora?
Comment on attachment 8842096 [details] [diff] [review]
Capture a strong reference to nsRootPresContext in EnsureEventualDidPaintEvent

Not taking the uplift from bug 1302071 so we shouldn't need this on 53 aurora. (It's only aurora for a couple more days)
Attachment #8842096 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Group: layout-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main54+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.