Closed
Bug 850672
Opened 12 years ago
Closed 12 years ago
use-after-poison with tables, -moz-perspective and transform [@ OverflowChangedTracker::Flush]
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla22
| Tracking | Status | |
|---|---|---|
| firefox21 | --- | unaffected |
| firefox22 | --- | fixed |
| firefox-esr17 | --- | unaffected |
| b2g18 | --- | unaffected |
People
(Reporter: miaubiz, Assigned: mattwoodrow)
References
Details
(5 keywords, Whiteboard: [asan][adv-main22-])
Attachments
(4 files, 1 obsolete file)
this:
<html>
<head>
<style>
#el0 {
display: table-cell;
}
#el2 {
transform: skew(0);
display: table;
float: left;
}
#el1 {
float: left;
}
.c0 {
float: left;
}
</style>
<script>
onload = function() {
el0=document.createElement('div')
el0.setAttribute('id','el0')
document.body.appendChild(el0)
el1=document.createElement('div')
el1.setAttribute('id','el1')
document.body.appendChild(el1)
el2=document.createElement('div')
el2.setAttribute('id','el2')
document.body.appendChild(el2)
document.body.offsetTop
document.styleSheets[0].insertRule("#el2 { -moz-perspective: 1px; } ", 0)
el0.setAttribute('class', 'c0')
document.body.offsetTop
}</script>
</head><body>
</body></html>
gives this:
==70831== ERROR: AddressSanitizer: use-after-poison on address 0x00011f429b98 at pc 0x104207bfb bp 0x7fff5fbeb570 sp 0x7fff5fbeb568
READ of size 8 at 0x00011f429b98 thread T0
#0 0x104207bfa in nsIFrame::StyleContext() const (in XUL) + 42
#1 0x104207af8 in nsIFrame::PresContext() const (in XUL) + 8
#2 0x1042b3671 in nsIFrame::Properties() const (in XUL) + 113
0x00011f429b98 is located 7000 bytes inside of 8192-byte region [0x00011f428040,0x00011f42a040)
allocated by thread T0 here:
#0 0x10000ec7c in 0x20000ec7c
#1 0x7fff93066152 in _asl_msg_test_expression (in libsystem_c.dylib) + 82
#2 0x7fff93066ba6 in _asl_msg_new_key_val_op (in libsystem_c.dylib) + 570
#3 0x1025a4e9a in PL_ArenaAllocate (in libplds4.dylib) + 682
|
||
Comment 3•12 years ago
|
||
mOverflowChangedTracker contains a destroyed frame. We do get a call to
nsCSSFrameConstructor::NotifyDestroyingFrame for that frame while it is in
the mOverflowChangedTracker list, but it appears the RemoveFrame call there
fails to remove it for some reason.
Severity: normal → critical
OS: Mac OS X → All
Hardware: x86 → All
Summary: use-after-poison with tables, -moz-perspective and transform → use-after-poison with tables, -moz-perspective and transform [@ OverflowChangedTracker::Flush]
Whiteboard: [asan]
|
||
Updated•12 years ago
|
Flags: sec-bounty?
Assignee: nobody → matt.woodrow
Assignee: matt.woodrow → matspal
|
|
||
Comment 4•12 years ago
|
||
Well, I got as far as OverflowChangedTracker::RemoveFrame
http://hg.mozilla.org/mozilla-central/annotate/b2636816c7fd/layout/base/RestyleTracker.h#l59
Is depth==0 supposed to find the entry for the frame anywhere in the SplayTree?
I think Matt is a better owner for this bug, he wrote all of this code afaict.
Assignee: matspal → matt.woodrow
|
||
Comment 5•12 years ago
|
||
Our current understanding is that frame-poisoning is an effective mitigation so not awarding a bug bounty
Flags: sec-bounty? → sec-bounty-
|
Assignee | |
Comment 6•12 years ago
|
||
Good catch Mats, that is exactly the issue.
This is a regression from bug 849263 I guess, since before that we were pretty much ignoring the depth parameter.
Attachment #727019 -
Flags: review?(roc)
|
|
||
Comment 7•12 years ago
|
||
Entry(aFrame, false) expands to Entry(aFrame, aFrame->GetDepthInFrameTree(), false).
Would it be worth trying to avoid doing GetDepthInFrameTree() twice?
Also, this is called unconditionally on every destroyed frame so it might be
worth doing an early return if mEntryList is empty? (to avoid doing
GetDepthInFrameTree() at all).
(In reply to Mats Palmgren [:mats] from comment #7)
> Entry(aFrame, false) expands to Entry(aFrame, aFrame->GetDepthInFrameTree(),
> false).
> Would it be worth trying to avoid doing GetDepthInFrameTree() twice?
Yes please
> Also, this is called unconditionally on every destroyed frame so it might be
> worth doing an early return if mEntryList is empty? (to avoid doing
> GetDepthInFrameTree() at all).
Yes!
|
|
Assignee | |
Comment 9•12 years ago
|
||
Good call. I took the liberty of fixing AddFrame in the same way too
Attachment #727019 -
Attachment is obsolete: true
Attachment #727019 -
Flags: review?(roc)
Attachment #727035 -
Flags: review?(roc)
Attachment #727035 -
Flags: review?(roc) → review+
|
|
Assignee | |
Comment 10•12 years ago
|
||
|
||
Comment 11•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
status-firefox22:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
|
|
||
Updated•12 years ago
|
status-b2g18:
--- → unaffected
status-firefox21:
--- → unaffected
status-firefox-esr17:
--- → unaffected
Keywords: regression
|
||
Updated•11 years ago
|
Whiteboard: [asan] → [asan][adv-main22-]
|
|
||
Updated•11 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•