Closed
Bug 876092
Opened 8 years ago
Closed 8 years ago
out of bounds stack read in mozilla::DisplayItemClip::IntersectWith
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla24
Tracking | Status | |
---|---|---|
firefox22 | --- | unaffected |
firefox23 | - | fixed |
firefox24 | - | fixed |
firefox-esr17 | --- | unaffected |
b2g18 | --- | unaffected |
b2g18-v1.0.0 | --- | unaffected |
b2g18-v1.0.1 | --- | unaffected |
b2g-v1.1hd | --- | unaffected |
People
(Reporter: miaubiz, Assigned: roc)
References
Details
(6 keywords, Whiteboard: [asan][adv-main23-])
Attachments
(6 files)
459 bytes,
text/plain
|
Details | |
15.52 KB,
text/plain
|
Details | |
2.31 KB,
text/plain
|
Details | |
29.05 KB,
text/html
|
Details | |
3.40 KB,
patch
|
mats
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
3.65 KB,
patch
|
mats
:
review+
|
Details | Diff | Splinter Review |
opening: <html> <head> <style> #s2::before { content: "a"; position: absolute; } #s1 { overflow: -moz-hidden-unscrollable; } #s3 { position: relative; } #s5 { position: absolute; } </style> </head> <body> <strike id="s1"> <strike id="s2"> <small id="s3"> <div> <div id="s4"></div> </strike> </div> </body> </html> crashes asan build on osx and linux like so: ==23010== ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff5fbec8a0 at pc 0x1044d89c3 bp 0x7fff5fbe8d30 sp 0x7fff5fbe8d28 READ of size 1 at 0x7fff5fbec8a0 thread T0 #0 0x1044d89c2 in mozilla::DisplayItemClip::IntersectWith(mozilla::DisplayItemClip const&) (in XUL) + 178 #1 0x1044dc007 in mozilla::DisplayListClipState::GetCurrentCombinedClip(nsDisplayListBuilder*) (in XUL) + 343 #2 0x1045ac194 in nsDisplayItem::nsDisplayItem(nsDisplayListBuilder*, nsIFrame*) (in XUL) + 116 #3 0x10482cfed in nsCharClipDisplayItem::nsCharClipDisplayItem(nsDisplayListBuilder*, nsIFrame*) (in XUL) + 13 Address 0x7fff5fbec8a0 is located at offset 256 in frame <_ZN12nsBlockFrame16BuildDisplayListEP20nsDisplayListBuilderRK6nsRectRK16nsDisplayListSet> of T0's stack: This frame has 10 object(s): [32, 36) 'drawnLines' [96, 112) 'ca' [160, 168) 'textOverflow'
Comment 3•8 years ago
|
||
Comment 4•8 years ago
|
||
ASan detects accessing a stack allocated DisplayItemClip (that has gone out of scope), the address to it is stored on the OutOfFlowDisplayDataProperty frame property. Here's a trace of what happens, with the frame tree at the end. nsIFrame::BuildDisplayListForChild aChild=0x7fffda316d50 (cyan) creates the AutoClipMultiple on the stack then calls MarkOutOfFlowFrameForDisplay which stores the address (pink) of AutoClipMultiple::mExtraClip in the abs.pos. child 0x7fffda3171f8 (red) OutOfFlowDisplayDataProperty->mContainingBlockClip. Then we return mkaing that property have a dangling pointer. Later we call BuildDisplayListForChild on the placeholder (blue) which looks up the property on the out-of-flow and try to use its mContainingBlockClip (see call stack).
Comment 5•8 years ago
|
||
DisplayItemClip is a simple POD-type so I think the worst that can happen is that we read random memory (likely from the stack) and use that to intersect a nsRect or something. So marking sec-low for now, until there's evidence that we WRITE to that object.
Severity: normal → critical
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [asan]
Comment 6•8 years ago
|
||
roc, can you take this? I don't like that we both create DisplayItemClips on the stack and that we have a frame property with a DisplayItemClip*. That seems like *really* fragile design to put it mildly ;-)
Assignee: nobody → roc
Updated•8 years ago
|
status-b2g18:
--- → unaffected
status-b2g18-v1.0.0:
--- → unaffected
status-b2g18-v1.0.1:
--- → unaffected
status-b2g-v1.1hd:
--- → unaffected
status-firefox21:
--- → unaffected
status-firefox22:
--- → unaffected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
status-firefox-esr17:
--- → unaffected
tracking-firefox23:
--- → ?
tracking-firefox24:
--- → ?
Flags: in-testsuite?
Comment 7•8 years ago
|
||
We don't track sec-low, and we only uplift sec-low bugs when there's something interesting going on.
Assignee | ||
Comment 8•8 years ago
|
||
It's kinda broken to have the abs-pos placeholder not be a descendant of the parent of its abs-pos frame. But OK...
status-b2g18:
unaffected → ---
status-b2g18-v1.0.0:
unaffected → ---
status-b2g18-v1.0.1:
unaffected → ---
status-b2g-v1.1hd:
unaffected → ---
status-firefox21:
unaffected → ---
status-firefox22:
unaffected → ---
status-firefox23:
affected → ---
status-firefox24:
affected → ---
status-firefox-esr17:
unaffected → ---
tracking-firefox23:
- → ---
tracking-firefox24:
- → ---
Assignee | ||
Updated•8 years ago
|
status-b2g18:
--- → unaffected
status-b2g18-v1.0.0:
--- → unaffected
status-b2g18-v1.0.1:
--- → unaffected
status-b2g-v1.1hd:
--- → unaffected
status-firefox22:
--- → unaffected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
status-firefox-esr17:
--- → unaffected
tracking-firefox23:
--- → -
tracking-firefox24:
--- → -
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #762539 -
Flags: review?(matspal)
Comment 10•8 years ago
|
||
Comment on attachment 762539 [details] [diff] [review] fix r=mats
Attachment #762539 -
Flags: review?(matspal) → review+
Assignee | ||
Comment 11•8 years ago
|
||
sec-low, only on Aurora, so I think we can just land this.
Assignee | ||
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2d55d17a0d8
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 762539 [details] [diff] [review] fix [Approval Request Comment] Bug caused by (feature/regressing bug #): 841192 User impact if declined: possible crash or weird rendering in odd cases Testing completed (on m-c, etc.): just landed Risk to taking this patch (and alternatives if risky): very low risk String or IDL/UUID changes made by this patch: none
Attachment #762539 -
Flags: approval-mozilla-aurora?
Comment 14•8 years ago
|
||
Push backed out for reftest failures: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ff486a6eb5c9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1ad2d8b5e7c9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8048b030fe6 https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f2d55d17a0d8
Assignee | ||
Comment 15•8 years ago
|
||
Looks like this caused the regression.
Assignee | ||
Comment 16•8 years ago
|
||
The test image looks like it's been overdrawn many times, so clipping is probably failing to work somewhere.
Assignee | ||
Comment 17•8 years ago
|
||
Attachment #763530 -
Flags: review?(matspal)
Assignee | ||
Comment 18•8 years ago
|
||
This is an existing bug that was somehow not visibly failing due to this bug.
Assignee | ||
Updated•8 years ago
|
Attachment #762539 -
Flags: approval-mozilla-aurora?
Comment 19•8 years ago
|
||
Comment on attachment 763530 [details] [diff] [review] fix failing reftest r=mats
Attachment #763530 -
Flags: review?(matspal) → review+
Assignee | ||
Comment 20•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fc64b1eeebd https://hg.mozilla.org/integration/mozilla-inbound/rev/194268f3bd8a
Comment 21•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7fc64b1eeebd https://hg.mozilla.org/mozilla-central/rev/194268f3bd8a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 762539 [details] [diff] [review] fix Review of attachment 762539 [details] [diff] [review]: ----------------------------------------------------------------- [Approval Request Comment] Bug caused by (feature/regressing bug #): 841192 User impact if declined: possible crash or weird rendering in odd cases Testing completed (on m-c, etc.): just landed Risk to taking this patch (and alternatives if risky): very low risk String or IDL/UUID changes made by this patch: none (This approval request is for both patches)
Attachment #762539 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
Attachment #762539 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 23•8 years ago
|
||
With both patches folded together. https://hg.mozilla.org/releases/mozilla-aurora/rev/b4489418e4b3
Updated•8 years ago
|
Flags: sec-bounty?
Whiteboard: [asan] → [asan][adv-main23-]
Comment 24•8 years ago
|
||
Does not qualify for the security bug bounty
Flags: sec-bounty? → sec-bounty-
Keywords: csec-bounds
Summary: stack buffer overflow with in mozilla::DisplayItemClip::IntersectWith → out of bounds stack read in mozilla::DisplayItemClip::IntersectWith
Updated•7 years ago
|
Group: core-security
Comment 25•7 years ago
|
||
Landed the crashtest: https://hg.mozilla.org/integration/mozilla-inbound/rev/77127ac8cc9f
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•