306663 - (stirdom) [meta] Bugs found by "Stir DOM" module of DOMFuzz

Status: RESOLVED FIXED

(Core :: Fuzzing, defect)

Description

[Jesse Ruderman]

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.

Comment 1

[Brendan Eich [:brendan]]

Fun.  This needs attention, pronto.  Who will own it?

/be

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: some additional table assertions

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.

Comment 3

[Jesse Ruderman]

The GetNifOrSpecialSibling crash might also be exploitable: TB8950615Y.

Comment 4

[Jesse Ruderman]

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.

Comment 5

[Jesse Ruderman]

I've seen Opera(Win), IE(Win), and Safari(Mac) crash with this bookmarklet.  I
haven't seen Firefox trunk crash yet.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Jesse, could you possibly try trunk builds to see when the crash disappeared on
trunk?

Comment 7

[Jesse Ruderman]

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

[Hixie (not reading bugmail)]

FWIW I eventually got a crash running that bookmarklet on this very page (the
bug report itself, I mean).

Comment 9

[Boris Zbarsky [:bzbarsky]]

> 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

[Boris Zbarsky [:bzbarsky]]

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

[Scott MacGregor]

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

[Boris Zbarsky [:bzbarsky]]

> 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).

Comment 13

[Asa Dotzler [:asa]]

going to take 256367 asap.

Comment 14

[Jesse Ruderman]

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

[Boris Zbarsky [:bzbarsky]]

Checked in bug 265367 on the 1.8 branch.

Comment 16

[Jesse Ruderman]

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.

Comment 17

[Jesse Ruderman]

See also bug 306939, the metabug for another crash-finding bookmarklet.

Comment 18

[Jesse Ruderman]

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.

Comment 19

[Jesse Ruderman]

How much do we care about assertion failures turned up by this kind of testing?
 Memory leaks?

Comment 20

[Boris Zbarsky [:bzbarsky]]

> 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

[Brendan Eich [:brendan]]

(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

Comment 22

[Jesse Ruderman]

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.

Comment 23

[Jesse Ruderman]

Putting new version of the bookmarklet in the URL field.  You can now control
the random number seed and several speed-related parameters.

Comment 24

[Jesse Ruderman]

I will retest http://www.xulplanet.com/tutorials/xulqa/treeimage.xul and other
tree examples after bug 307298 is fixed.

Comment 25

[Jesse Ruderman]

I will retest http://www.carto.net/papers/svg/samples/text.svg after bug 307314
is fixed.

Comment 26

[Jesse Ruderman]

I will retest http://www.carto.net/papers/svg/samples/symbol.svg after bug
307322 is fixed.

Comment 27

[Jesse Ruderman]

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 :)

Comment 28

[Jesse Ruderman]

I will retest everything under
http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-index.html once bug
307444 is fixed.

Comment 29

[Jesse Ruderman]

... and http://www.svgbasics.com/.

Comment 30

[Jesse Ruderman]

I will retest http://www.svgbasics.com/examples/marker_ex1.svg after bug 307470
is fixed.

Comment 31

[Jesse Ruderman]

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.

Comment 32

[Jesse Ruderman]

Attachment: Stir DOM 1.5 (for pasting into testcases)

Comment 33

[Jesse Ruderman]

Attachment: Stir DOM 1.5.1 (for pasting into testcases)

Comment 34

[Jesse Ruderman]

Attachment: Stir DOM 1.6 (for pasting into testcases)

* 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.

Comment 35

[Jesse Ruderman]

Attachment: Suggested URLs for automatic testing with Stir DOM

List includes URLs I found crashes on and URLs that use rarely-used Gecko
features.

Comment 36

[Mats Palmgren (inactive)]

FYI, I'm not authorized to access the majority of the bugs you're referring,
in case you want any help on those...

Comment 37

[Jesse Ruderman]

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

[Asa Dotzler [:asa]]

We'll evaluate each crash fix as they come in. No longer blocking on tracking bugs.

Comment 39

[Jesse Ruderman]

Attachment: Stir DOM recorder

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.

Comment 40

[Bob Clary [:bc] (inactive)]

Attachment: zip file of recent stirdom logs

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.

Comment 41

[Jesse Ruderman]

Attachment: Stir DOM recorder

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.

Comment 42

[Jesse Ruderman]

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.

Comment 43

[Jesse Ruderman]

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.

Comment 44

[Jesse Ruderman]

Attachment: Stir DOM 1.8 (requires fuzz.js)

* 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.

Comment 45

[Jesse Ruderman]

Attachment: Stir DOM 2.0

Updated for fuzz.js 2.0.

Comment 46

[Jesse Ruderman]

Attachment: Stir DOM 3.0

Comment 47

[Daniel Veditz [:dveditz]]

Shouldn't have security bugs assigned to nobody. Jesse can own his test bugs

Comment 48

[Jesse Ruderman]

Comment on attachment 242967 [details]
Stir DOM 3.0

New version in bug 339948.

Comment 49

[Jesse Ruderman]

This bookmarklet evolved into a DOMFuzz module, and is part of today's public release of DOMFuzz:

https://github.com/MozillaSecurity/funfuzz/blob/master/dom/fuzzer/modules/stir-dom.js

Comment 50

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:decoder, could you have a look please?
For more information, please visit auto_nag documentation.