Bug 460803 (CVE-2008-5500)

[FIX]PresShell::InitialReflow "ASSERTION: Why are we being called?" with XUL iframe

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
Mac OS X
assertion, testcase, verified1.8.1.19, verified1.9.0.5
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.0.5 +
blocking1.8.1.19 +
blocking1.8.0.next +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
Created attachment 343940 [details]
testcase (has external dependencies)

About 50% of the time, loading the testcase triggers these six assertions:

1. ###!!! ASSERTION: Why are we being called?: '!mDidInitialReflow', file layout/base/nsPresShell.cpp, line 2373

2. ###!!! ASSERTION: initial containing block already created: 'nsnull == mInitialContainingBlock', file layout/base/nsCSSFrameConstructor.cpp, line 8760

3. ###!!! ASSERTION: Already have an undisplayed context entry for aContent: '!GetUndisplayedContent(aContent)', file layout/base/nsFrameManager.cpp, line 574

4. ###!!! ASSERTION: unexpected mInitialContainingBlock: 'processChildren ? !mInitialContainingBlock : mInitialContainingBlock == contentFrame', file layout/base/nsCSSFrameConstructor.cpp, line 4308

5. ###!!! ASSERTION: unexpected second call to SetInitialChildList: 'Not Reached', file layout/generic/nsContainerFrame.cpp, line 111

6. ###!!! ASSERTION: Why is the root in mDirtyRoots already?: 'mDirtyRoots.IndexOf(rootFrame) == -1', file layout/base/nsPresShell.cpp, line 2462

and then closing it triggers:

7. ###!!! ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file layout/base/nsPresShell.cpp, line 676
(Assignee)

Comment 1

9 years ago
Good stuff.  We get into nsMediaDocument::StartLayout when OnStartRequest fires.  We grab the presshell, start layout on it.  Then we do a synchronous paint, which flushes pending notifications.  That triggers a frame reconstruct in the parent document (from the pending restyle), which blows away the presshell we just started layout, creates a new one, and starts layout on it.  Then we return from the paint call, move on to the next presshell for the document (the newly-created one!) and proceed to try starting layout on it.  Then we lose.
(Assignee)

Comment 2

9 years ago
Created attachment 343977 [details] [diff] [review]
Do it like the sinks
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #343977 - Flags: superreview?(jonas)
Attachment #343977 - Flags: review?(jonas)
(Assignee)

Comment 3

9 years ago
The key here is that the image needs to come in after the attr set happened but before the restyle is processed.  Fun.  :(  Not sure how to guarantee that in a test.
Summary: PresShell::InitialReflow "ASSERTION: Why are we being called?" with XUL iframe → [FIX]PresShell::InitialReflow "ASSERTION: Why are we being called?" with XUL iframe
Attachment #343977 - Flags: superreview?(jonas)
Attachment #343977 - Flags: superreview+
Attachment #343977 - Flags: review?(jonas)
Attachment #343977 - Flags: review+
(Assignee)

Comment 4

9 years ago
Pushed changeset f05dd60fa3ed.
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Attachment #343977 - Flags: approval1.9.0.4?
(Assignee)

Comment 5

9 years ago
Comment on attachment 343977 [details] [diff] [review]
Do it like the sinks

I do think that we should take this on branch, but I'd request apprval11.9.0.5 (not .4) if I could...
(Assignee)

Comment 6

9 years ago
Created attachment 344162 [details] [diff] [review]
1.8 branch patch

Again, I'd probably try for the next branch release not this very next one.
Attachment #344162 - Flags: approval1.8.1.18?
(Assignee)

Comment 7

9 years ago
Created attachment 344163 [details] [diff] [review]
1.8 branch non-ugly patch
Attachment #344162 - Attachment is obsolete: true
Attachment #344163 - Flags: approval1.8.1.18?
Attachment #344162 - Flags: approval1.8.1.18?
Attachment #343977 - Flags: approval1.9.0.4? → approval1.9.0.5?
Attachment #344163 - Flags: approval1.8.1.18? → approval1.8.1.19?
Flags: blocking1.9.0.5+
Flags: blocking1.8.1.19+
Whiteboard: [sg:critical?]
Comment on attachment 343977 [details] [diff] [review]
Do it like the sinks

approved for 1.9.0.5 and 1.8.1.19, a=dveditz for release-drivers

(please watch tinderbox for the tree to open before landing)
Attachment #343977 - Flags: approval1.9.0.5? → approval1.9.0.5+
Attachment #344163 - Flags: approval1.8.1.19? → approval1.8.1.19+
(Assignee)

Comment 9

9 years ago
Fixed on both branches.
Keywords: fixed1.8.1.19, fixed1.9.0.5
verified fixed for 1.8.1.19 - using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.20pre) Gecko/2008120304 Firefox/2.0.0.20pre (1.8.1 Debug Build) and the Testcase from Jesse.

I do not see the Assertions from Comment #0, only ###!!! ASSERTION: Adding child where we already have a child?  This will likely misbehave: 'Error', file /work/mozilla/builds/1.8.1/mozilla/xpfe/components/shistory/src/nsSHEntry.cpp, line 580, which is a different Bug.

--> Verified fixed 1.8.1.19
Keywords: fixed1.8.1.19 → verified1.8.1.19
also verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2008120309 Firefox/3.0.6pre (Debug Build) and the testcase from Jesse. I do not see the Assertions from Comment #0 when using the Testcase

--> Verified 1.9.0.5
Keywords: fixed1.9.0.5 → verified1.9.0.5

Comment 12

9 years ago
Comment on attachment 344163 [details] [diff] [review]
1.8 branch non-ugly patch

a=asac for 1.8.0
Attachment #344163 - Flags: approval1.8.0.next+

Updated

9 years ago
Flags: blocking1.8.0.next+
Alias: CVE-2008-5500
Group: core-security
You need to log in before you can comment on or make changes to this bug.