Closed
Bug 306663
(stirdom)
Opened 19 years ago
Closed 3 years ago
[meta] Bugs found by "Stir DOM" module of DOMFuzz
Categories
(Core :: Fuzzing, defect)
Core
Fuzzing
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Unassigned)
References
(Depends on 14 open bugs)
Details
(Keywords: meta, sec-other, Whiteboard: [sg:nse] meta)
Attachments
(3 files, 8 obsolete files)
Steps to reproduce:
1. Go to a small web page, such as http://www.google.com/.
2. Use the bookmarklet in this bug's URL field.
3. Wait a while.
Result: more often than not, Firefox crashes with a minute or two.
Here are the crashes I've seen so far with the bookmarklet:
nsStyleContext::GetStyleData()
TB8922845W, TB8923024X, TB8923158K, TB8923275Q, TB8923389M, TB8923438Z
GetNifOrSpecialSibling()
TB8923432H
nsCSSFrameConstructor::GetFloatContainingBlock()
TB8922738K, TB8923005W, TB8923133W*
The starred crash report is particularly scary because it occurred at a "random
address" with SIGILL. According to the libc manual, "SIGILL typically indicates
that the executable file is corrupted, or that you are trying to execute data."
I'm filing this as a security-sensitive bug because I have already found a
potential security hole with it. I will keep the bookmarklet secret for now as
well, because someone with the bookmarklet might be able to rediscover the
scarier crashes.
I can make a more deterministic version of the bookmarklet if that would make it
easier to make a testcase for each crash it finds.
Reporter | ||
Updated•19 years ago
|
Group: security
Comment 1•19 years ago
|
||
Fun. This needs attention, pronto. Who will own it?
/be
Flags: blocking1.8b5+
Flags: blocking1.8b4?
These additional assertions prevent one problem from showing up 20 frames down
the stack when we do a bogus reflow with a bogus 0 availableHeight -- they at
least cause the problem to show up as an assertion pretty close to where it
happens. I think the underlying problem here is bug 203923. But I don't know
whether it's related to any of the crashes, which I haven't hit yet.
Attachment #194596 -
Flags: review?(bernd.mielke)
Reporter | ||
Comment 3•19 years ago
|
||
The GetNifOrSpecialSibling crash might also be exploitable: TB8950615Y.
Reporter | ||
Comment 4•19 years ago
|
||
All of the crashes I've found so far happen on the Gecko 1.8 branch but not on
trunk. I suspect they all have the same underlying cause, which was fixed on
the trunk after the branch point.
Reporter | ||
Comment 5•19 years ago
|
||
I've seen Opera(Win), IE(Win), and Safari(Mac) crash with this bookmarklet. I
haven't seen Firefox trunk crash yet.
Comment 6•19 years ago
|
||
Jesse, could you possibly try trunk builds to see when the crash disappeared on
trunk?
Reporter | ||
Comment 7•19 years ago
|
||
The crashes for the testcases on all four bugs (bug 306782, bug 306787, bug
306789, bug 306798) disappeared between Aug 16 and Aug 17 trunk nightlies.
2005-08-16-07-trunk - crash (all four testcases)
2005-08-17-09-trunk - no crash (all four testcases)
Bug 265367 was checked in between those builds. Could it have fixed these crashes?
Comment 8•19 years ago
|
||
FWIW I eventually got a crash running that bookmarklet on this very page (the
bug report itself, I mean).
Comment 9•19 years ago
|
||
> Bug 265367 was checked in between those builds. Could it have fixed these
> crashes?
Possibly, yes. It would certainly help on any page that has an animated image
in a very direct way; for other pages I'm not sure how it could prevent crashes,
but I wouldn't be too surprised.
Also bug 289377 landed in the same range. And so did bug 303484.
Comment 10•19 years ago
|
||
So I found a general case where the patch for bug 265367 would prevent a crash.
Say we have nodes A, B, and C and that the frame for node A is a leaf (eg A is
an <img>). Consider the following sequence of operations, starting with a state
when A is in the document and hence has a frame (called A').
1) Insert B as a child of A. This creates a frame B' for B, sets the parent of
B' to be A', sets B' as the primary frame for B. Then we try to append B'
to the frame list of A'. This throws, but the error return is ignored. So
B' is not in the child list of A' (since A' has no child list).
2) Move A to a different place in the DOM. This destroys A' and all its
descendants. Since B' is not a child of A', B' is NOT destroyed.
3) Insert C as a child of B. This tries to create a frame C' for C, with B' as
its parent (since B' is what we got when we did a primary frame lookup on
B). We start looking up things like the containing blocks we should use,
which means we walk the parent chain of B'. But the mParent pointer of B'
points to the now-deleted A' frame, or in other words to some random memory.
We might not crash there right away, since the random memory comes from an
arena and hence is probably still allocated to our process; if we've
allocated over it again, it won't even be marked with 0xdddddddd.
The alternative to the patch for bug 265367 is to actually check rv for all
cases when we're inserting or appending frames and clean up if it fails (at the
very least clean up the primary frame map). This is not completely trivial to
do right in full generality (i.e. with destruction of the not-appended frames),
I suspect, and the patch for bug 265367 is pretty safe given that there have
been no regressions reported yet, so maybe we should just take bug 265367 on the
branch and also add the rv checks on trunk just in case?
Comment 11•19 years ago
|
||
Jesse, what did 1.0.x do with this test case? Is this crash a regression over
1.0? Fixing this crash may mean taking a pretty large change to gecko, Bug
265367 which scares me after a looking at it briefly. I'd like to see if this
crash is new or not and how many crash reports we are seeing in talkback before
making a decision on this and 265367.
Comment 12•19 years ago
|
||
> what did 1.0.x do with this test case?
Crashed.
It also crashed on the testcases in bug 291022 and bug 291513, of course. ;)
And I think the reason Jesse is worried is not just because it's a crash but a
crash likely to be exploitable.
For what it's worth, the change for bug 265367 is a good bit safer than a lot of
the other stuff we've been landing on branch for a while now... At least in my
opinion. The other approach (of checking rv on the inserts/appends) would
actually be less safe, imo (and would certainly be less tested; I was running
with the patch in bug 265367 for three months and didn't see any issues).
Reporter | ||
Comment 14•19 years ago
|
||
I made a small change to the bookmarklet so it would work on XML pages:
- var myRoot = document.body;
+ var myRoot = document.body || document.documentElement;
Comment 15•19 years ago
|
||
Checked in bug 265367 on the 1.8 branch.
Reporter | ||
Comment 16•19 years ago
|
||
Aaron, tomorrow's branch build won't crash much when you use this bookmarklet on
HTML pages, so you can start testing accessiblity code and/or screen readers
with this bookmarklet. See the testcase in e.g. bug 306915 for the version that
includes its own random number generator for reproducibility.
Reporter | ||
Comment 17•19 years ago
|
||
See also bug 306939, the metabug for another crash-finding bookmarklet.
Reporter | ||
Comment 18•19 years ago
|
||
I should do another round of testing on HTML now that I can avoid the crashes
caused by bug 265367.
Bug 306902 shows up often (with many different seeds) on mixed HTML-and-MathML.
I'll do another round of testing on mixed HTML-and-MathML pages once it is fixed.
Bug 306911 or bug 306915 happens almost immediately when I run the bookmarklet
on a XUL page. I'll do another round of testing on XUL pages once they are fixed.
Reporter | ||
Comment 19•19 years ago
|
||
How much do we care about assertion failures turned up by this kind of testing?
Memory leaks?
Comment 20•19 years ago
|
||
> How much do we care about assertion failures turned up by this kind of testing?
I care a good bit.
> Memory leaks?
Same, esp. if we can get a non-randomized testcase... ;)
Comment 21•19 years ago
|
||
(In reply to comment #20)
> > How much do we care about assertion failures turned up by this kind of testing?
>
> I care a good bit.
I proposed that we make assertions fatal for 1.9, see
http://weblogs.mozillazine.org/roadmap/archives/2005_01.html. dbaron suggested
doing this first by setting the XPCOM_DEBUG_BREAK envariable for some tinderbox
machines, to build up cross-platform testing coverage. That's a good idea, and
I think we will do this starting tomorrow, or as soon as we can.
/be
Reporter | ||
Comment 22•19 years ago
|
||
I'm seeing crashes at DoDeletingFrameSubtree (bug 307277) and/or
nsBlockFrame::Destroy (bug 307280) on multiple sites:
http://www.csszengarden.com/, http://www.43things.com/, http://www.w3.org/Style/.
Once those bugs are fixed, I'll retest those sites.
Reporter | ||
Comment 23•19 years ago
|
||
Putting new version of the bookmarklet in the URL field. You can now control
the random number seed and several speed-related parameters.
Reporter | ||
Comment 24•19 years ago
|
||
I will retest http://www.xulplanet.com/tutorials/xulqa/treeimage.xul and other
tree examples after bug 307298 is fixed.
Reporter | ||
Updated•19 years ago
|
Alias: stirdom
Reporter | ||
Comment 25•19 years ago
|
||
I will retest http://www.carto.net/papers/svg/samples/text.svg after bug 307314
is fixed.
Reporter | ||
Comment 26•19 years ago
|
||
I will retest http://www.carto.net/papers/svg/samples/symbol.svg after bug
307322 is fixed.
Depends on: 307394
Reporter | ||
Comment 27•19 years ago
|
||
I retested http://www.carto.net/papers/svg/samples/text.svg on trunk (now that
bug 307314 is fixed) and didn't hit any more crashes :)
Reporter | ||
Comment 28•19 years ago
|
||
I will retest everything under
http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-index.html once bug
307444 is fixed.
Reporter | ||
Comment 29•19 years ago
|
||
... and http://www.svgbasics.com/.
Reporter | ||
Comment 30•19 years ago
|
||
I will retest http://www.svgbasics.com/examples/marker_ex1.svg after bug 307470
is fixed.
Reporter | ||
Comment 31•19 years ago
|
||
I will retest http://www.w3.org/TR/SVG/images/filters/filters01.svg on Windows
after bug 307565 is fixed with seeds 0 and 1, at least.
Reporter | ||
Comment 32•19 years ago
|
||
Reporter | ||
Comment 33•19 years ago
|
||
Attachment #195379 -
Attachment is obsolete: true
Reporter | ||
Updated•19 years ago
|
Reporter | ||
Updated•19 years ago
|
Reporter | ||
Comment 34•19 years ago
|
||
* Added discovery of nodes in frames, iframes, and <embed>s containing SVG.
* Added discovery of XBL anonymous children.
* Made it use chained setTimeout instead of setInterval to improve
responsiveness when slow.
* Increased the default speed.
* Commented out the "javascript:" at the top of the non-bookmarklet version
because it caused a parse error in Safari.
Attachment #195381 -
Attachment is obsolete: true
Reporter | ||
Comment 35•19 years ago
|
||
List includes URLs I found crashes on and URLs that use rarely-used Gecko
features.
Reporter | ||
Updated•19 years ago
|
Updated•19 years ago
|
Attachment #194596 -
Flags: review?(bernd.mielke) → review+
Comment 36•19 years ago
|
||
FYI, I'm not authorized to access the majority of the bugs you're referring,
in case you want any help on those...
Reporter | ||
Updated•19 years ago
|
Reporter | ||
Comment 37•19 years ago
|
||
There are five remaining StirDOM crashes that look exploitable to me:
* Tables: bug 308752
* DOM+UI: bug 307992, bug 307989
* MathML: bug 309120, bug 307826
There are several types of content for which Stir DOM runs into the same crash
often enough that it's hard to tell whether there are other crashes:
* XBL: bug 308120
* XUL grids: bug 306911
* XUL trees: bug 291531, bug 309732
* SVG: bug 307444
Based on what bz said in a security-group@ email, it sounds like we can't fix
the XUL crashes for Firefox 1.5 without making Firefox 1.5 slip. I'm ok with
postponing the release of Stir DOM until the next version of Firefox and hoping
nobody else thinks of doing the same kind of fuzz testing on web browsers. It
would be nice if the already-known, exploitable-looking crashes were fixed for
Firefox 1.5, though.
Gecko has a hidden pref for disabling SVG for use during security firedrills.
It might be worthwhile to add similar prefs for disabling:
* XUL in non-chrome
* MathML
* XBL in non-chrome (including <marquee> and <xul:button>)
Comment 38•19 years ago
|
||
We'll evaluate each crash fix as they come in. No longer blocking on tracking bugs.
Flags: blocking1.8b5+ → blocking1.8b5-
Reporter | ||
Comment 39•19 years ago
|
||
This is one of the tools I use to reduce Stir DOM testcases. Its output is
intended to be pasted back in, replacing the first two lines of the script.
Updated•19 years ago
|
Whiteboard: [sg:investigate]
Updated•19 years ago
|
Flags: blocking1.8.0.1?
Comment 40•19 years ago
|
||
crashes or timeouts found in this run appear as
stirdom: url?fuzz=parms...
The test loads the page (with the querystring), then runs the stirdom
bookmarklet with the specified parameters. You can copy/paste the parameters
from the query string directly into the stirdom input prompt.
The end of each line identifies the machine, the date the test run began and
the build which was tested. For example,
prunessh/2005-12-17-02-34-33-firefox-1.5-build-dbg-1.8_2005121411.log
was run on prune (a windows machine), on Dec 17, using a 1.8 debug build built
on 2005-12-14-11.
You can reproduce each test case by loading the url including the query string,
then running stirdom with the appropriate parameter.
Reporter | ||
Comment 41•19 years ago
|
||
Changes made to both Stir DOM and Random Styles recorders:
1. Make it record information about chunks/intervals so that
(a) it can record the equivalent of a nonzero "number of changes to do immediately" in the bookmarklet.
(b) while reducing, the chunk boundaries don't move.
2. Make it work with both XML and HTML without requiring separate versions.
3. Improve the instructions.
Attachment #198555 -
Attachment is obsolete: true
Updated•19 years ago
|
Flags: blocking1.8.0.1? → blocking1.8.0.1+
Whiteboard: [sg:investigate] → [sg:nse] meta
Updated•19 years ago
|
Flags: blocking1.8.0.1+
Reporter | ||
Comment 42•19 years ago
|
||
Stupid bug:
if(n.getSVGDocument){try{addNodes(n.getSVGDocument());}catch(ex){}}
should check that n.getSVGDocument() is non-null before recursing. I'm not fixing it (yet) because I'm lazy, and because it would change the behavior of the script on any page that uses Flash and I want to avoid changing its behavior too much.
Reporter | ||
Comment 43•19 years ago
|
||
Stir DOM and some other fuzzers will need changes once bug 47903, "WRONG_DOCUMENT_ERR not being thrown", is fixed. I'm not fixing Stir DOM now because I'm hoping to use adoptNode (bug 330677) rather than importNode.
Reporter | ||
Comment 44•19 years ago
|
||
* Converted it to use fuzz.js (see bug 339948).
* Turned off recursing into XBL for now.
* No longer uses separate versions for bookmarklet-source and recording :)
* Various small changes.
Attachment #196006 -
Attachment is obsolete: true
Attachment #207402 -
Attachment is obsolete: true
Reporter | ||
Updated•19 years ago
|
Reporter | ||
Comment 45•18 years ago
|
||
Updated for fuzz.js 2.0.
Attachment #224049 -
Attachment is obsolete: true
Reporter | ||
Comment 46•18 years ago
|
||
Attachment #226744 -
Attachment is obsolete: true
Comment 47•18 years ago
|
||
Shouldn't have security bugs assigned to nobody. Jesse can own his test bugs
Assignee: nobody → jruderman
Reporter | ||
Updated•18 years ago
|
Summary: Crashes found by Jesse's "Stir DOM" bookmarklet → Bugs found by Jesse's "Stir DOM" bookmarklet
Reporter | ||
Updated•18 years ago
|
Reporter | ||
Comment 48•18 years ago
|
||
Comment on attachment 242967 [details]
Stir DOM 3.0
New version in bug 339948.
Attachment #242967 -
Attachment is obsolete: true
Reporter | ||
Updated•18 years ago
|
Severity: critical → normal
Version: 1.8 Branch → Trunk
Reporter |
Description
•