Closed Bug 1233259 Opened 4 years ago Closed 4 years ago

Heap-use-after-free [@ nsDocument::NotifyMediaFeatureValuesChanged] with srcset

Categories

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

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox44 --- unaffected
firefox45 + fixed
firefox46 + fixed
firefox-esr38 --- unaffected

People

(Reporter: jruderman, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [post-critsmash-triage]don't unhide until bug 1240763 is fixed on ESR38)

Attachments

(3 files)

Attached file manual testcase
1. Load the testcase
2. Trigger cycle collection (press "CC" button in about:memory)
3. Zoom in on the testcase (press ⌘+ back in its tab)

Heap-use-after-free [@ nsDocument::NotifyMediaFeatureValuesChanged]
Attached file ASan stacks
Component: Layout → DOM
Assignee: nobody → bugs
Keywords: sec-high
We're missing mResponsiveContent from nsDocument's traverse etc. methods.
Attached patch patchSplinter Review
Attachment #8699287 - Flags: review?(michael)
(In reply to Josh Matthews [:jdm] from comment #2)
> We're missing mResponsiveContent from nsDocument's traverse etc. methods.

Nope. mResponsiveContent keeps raw pointers to nsIContent objects.
Comment on attachment 8699287 [details] [diff] [review]
patch

Review of attachment 8699287 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for figuring this out :)
Attachment #8699287 - Flags: review?(michael) → review+
Comment on attachment 8699287 [details] [diff] [review]
patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not sure about exploit, but the issue is pretty clear.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Commit message could be

Which older supported branches are affected by this flaw?
nightly, aurora

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch seems to apply to Aurora just fine

How likely is this patch to cause regressions; how much testing does it need?
Should be very safe.
Attachment #8699287 - Flags: sec-approval?
Attachment #8699287 - Flags: approval-mozilla-aurora?
(In reply to Olli Pettay [:smaug] from comment #6) 
> Do comments in the patch, the check-in comment, or tests included in the
> patch paint a bulls-eye on the security problem?
> Commit message could be
"Bug 1233259, only in-document images should respond to viewport changes, r=mystor"
Wait, am I reading the spec wrong.
Comment on attachment 8699287 [details] [diff] [review]
patch

(I need to figure out what the spec tries to say here)
Attachment #8699287 - Flags: sec-approval?
Attachment #8699287 - Flags: approval-mozilla-aurora?
Comment on attachment 8699287 [details] [diff] [review]
patch

Ok, the spec is about to be clarified and the patch gives that behavior 
https://github.com/whatwg/html/issues/414
Attachment #8699287 - Flags: sec-approval?
Attachment #8699287 - Flags: approval-mozilla-aurora?
(I marked this sec-high rather than critical because it seems like it may require user interaction. That said, resizing the window or whatever isn't too much of a burden so it could also be critical I guess.)
Comment on attachment 8699287 [details] [diff] [review]
patch

Approvals given.
Attachment #8699287 - Flags: sec-approval?
Attachment #8699287 - Flags: sec-approval+
Attachment #8699287 - Flags: approval-mozilla-aurora?
Attachment #8699287 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/f1d406297b87
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Group: layout-core-security → core-security-release
Duplicate of this bug: 1240763
This says FF 44 is unaffected but I believe bug 1240763 is the same symptom of a crash caused by responsive content in an FF 43 build.
Don't have access to bug 1240763
Group: core-security-release
Group: dom-core-security
Blocks: 1240763
Whiteboard: don't unhide until bug 1240763 is fixed on ESR38
Group: dom-core-security → core-security-release
Duplicate of this bug: 1240763
Whiteboard: don't unhide until bug 1240763 is fixed on ESR38 → [post-critsmash-triage]don't unhide until bug 1240763 is fixed on ESR38
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.