CVE-2006-0294 [FIX]Crash with evil testcase, switching from position:relative to static

VERIFIED FIXED in mozilla1.9alpha1



12 years ago
8 years ago


(Reporter: Martijn Wargers (dead), Assigned: bz)


(5 keywords)

crash, regression, testcase, verified1.8.0.1, verified1.8.1
Bug Flags:
blocking1.7.13 -
blocking-aviary1.0.8 -
blocking1.8.1 +
blocking1.8.0.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [sg:critical?])


(3 attachments)



12 years ago
See upcoming testcase, when clicking on the button, current Mozilla builds crash.
This doesn't happen in Mozilla1.7.

Talkback ID: TB12309372W
nsIView::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsView.cpp, line 307]
nsSplittableFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsSplittableFrame.cpp, line 71]
nsPositionedInlineFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 1052]
nsFrameList::DestroyFrames  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsFrameList.cpp, line 138]
nsPositionedInlineFrame::Destroy  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsInlineFrame.cpp, line 1052]
nsBlockFrame::RemoveFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsBlockFrame.cpp, line 5516]
nsFrameManager::RemoveFrame  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsFrameManager.cpp, line 704]
nsCSSFrameConstructor::ContentRemoved  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9946]
nsCSSFrameConstructor::RecreateFramesForContent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 11495]
nsCSSFrameConstructor::RestyleElement  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 10377]
nsCSSFrameConstructor::ProcessOneRestyle  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13211]
nsCSSFrameConstructor::ProcessPendingRestyles  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13259]
nsCSSFrameConstructor::RestyleEvent::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 13324]
SHELL32.dll + 0x520c24 (0x778b0c24)

Comment 1

12 years ago
Created attachment 204292 [details]


12 years ago
Severity: normal → critical

Comment 2

12 years ago
Well, this seems to have regressed between 2005-06-20 and 2005-06-22:

I don't have really an idea which bug could have caused the regression.

Comment 3

12 years ago
testcase crashes 1.5rc3 on OS/2
OS: Windows XP → All

Comment 4

12 years ago
Created attachment 204297 [details]
valgrind log

I crashed with builds back to 2005-06-10 but then couldn't crash with any build.  valgrind caught it trying to use already-freed memory.
I see this only when we process the restyle for the inner <q> first.  Then when we're processing the restyle for the outer <q> we get to destroying the brand-new frame the inner restyle created for the positioned <span> and crash when trying to delete its view...
The inner frame shouldn't have a view anymore ... did we somehow fail to remove the view from its parent when we reframed the inner element?
Created attachment 204322 [details] [diff] [review]
This should fix it...

This looks like a regression from bug 292116 -- changing the parent means we get a mismatch between the frame and view trees....
Attachment #204322 - Flags: superreview?(roc)
Attachment #204322 - Flags: review?(roc)


12 years ago
Assignee: nobody → bzbarsky
Group: security
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
Priority: -- → P1
Summary: Crash with evil testcase, switching from position:relative to static → [FIX]Crash with evil testcase, switching from position:relative to static
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 204322 [details] [diff] [review]
This should fix it...

don't you need to initialize appendAfterFrame to nsnull to avoid a 'possible use of uninitialized' warning?
Attachment #204322 - Flags: superreview?(roc)
Attachment #204322 - Flags: superreview+
Attachment #204322 - Flags: review?(roc)
Attachment #204322 - Flags: review+
I didn't get those warnings when I compiled, so I assumed that gcc is finally sorta-smart enough...

I'll init them if warnings appear when I check in.
Last Resolved: 12 years ago
Resolution: --- → FIXED
Comment on attachment 204322 [details] [diff] [review]
This should fix it...

This is a double-delete, hence potentially exploitable... I think we should take this on the 1.8 branch, after it bakes on the trunk a tad.
Attachment #204322 - Flags: approval1.8.0.1?
I can't speak to any *potential* regressions, but the crash itself is certainly fixed in trunk build 2005-11-29-09 of SeaMonkey trunk on Windows XP.

Verified (trunk)

Comment 13

12 years ago
Comment on attachment 204322 [details] [diff] [review]
This should fix it...

Please land on 1.8 and 1.8.1 branches.
Attachment #204322 - Flags: approval1.8.1+
Attachment #204322 - Flags: approval1.8.0.1?
Attachment #204322 - Flags: approval1.8.0.1+
Could someone please land this for me?  If not, I'll do it in early January...
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
*** Committing to MOZILLA_1_8_BRANCH... 
new revision:; previous revision:
new revision: 1.1110.6.13; previous revision: 1.1110.6.12

*** Committing layout/base/nsCSSFrameConstructor.cpp on MOZILLA_1_8_0_BRANCH... 
new revision: 1.1110.; previous revision: 1.1110.6.12
*** Committing layout/base/nsCSSFrameConstructor.h on MOZILLA_1_8_0_BRANCH... 
new revision:; previous revision:
Keywords: fixed1.8.0.1, fixed1.8.1

Comment 16

12 years ago
v.fixed on with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060111 Firefox/, no crash with testcase in comment #1
Keywords: fixed1.8.0.1 → verified1.8.0.1

Comment 17

12 years ago
v test case does not crash, 1.8.1, 1.9a1 windows/linux
Keywords: fixed1.8.1 → verified1.8.1


12 years ago
Flags: testcase?


12 years ago
Flags: testcase? → testcase+
Whiteboard: [sg:critical?]
Flags: blocking1.7.13-
Flags: blocking-aviary1.0.8-


12 years ago
Summary: [FIX]Crash with evil testcase, switching from position:relative to static → CVE-2006-0294 [FIX]Crash with evil testcase, switching from position:relative to static
Group: security


11 years ago
Flags: in-testsuite+ → in-testsuite?

Comment 18

8 years ago
crash test landed
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.