Open Bug 306663 (stirdom) Opened 15 years ago Updated 2 years ago

Bugs found by "Stir DOM" module of DOMFuzz

Categories

(Core :: Fuzzing, defect)

defect
Not set
normal

Tracking

()

People

(Reporter: jruderman, Assigned: jruderman)

References

(Depends on 15 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.
Group: security
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.
The GetNifOrSpecialSibling crash might also be exploitable: TB8950615Y.
Depends on: 306782
Depends on: 306789
Depends on: 306798
Depends on: 306787
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.
I've seen Opera(Win), IE(Win), and Safari(Mac) crash with this bookmarklet.  I
haven't seen Firefox trunk crash yet.
Jesse, could you possibly try trunk builds to see when the crash disappeared on
trunk?
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?
FWIW I eventually got a crash running that bookmarklet on this very page (the
bug report itself, I mean).
> 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.
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?
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.

> 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).
going to take 256367 asap.
Flags: blocking1.8b4?
Depends on: 265367
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;
Depends on: 306902
Depends on: 306911
Depends on: 306915
Checked in bug 265367 on the 1.8 branch.
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.
See also bug 306939, the metabug for another crash-finding bookmarklet.
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.
How much do we care about assertion failures turned up by this kind of testing?
 Memory leaks?
> 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... ;)
(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
Depends on: 307277
Depends on: 307280
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.
Putting new version of the bookmarklet in the URL field.  You can now control
the random number seed and several speed-related parameters.
Depends on: 307298
I will retest http://www.xulplanet.com/tutorials/xulqa/treeimage.xul and other
tree examples after bug 307298 is fixed.
Alias: stirdom
Depends on: 307314
I will retest http://www.carto.net/papers/svg/samples/text.svg after bug 307314
is fixed.
Depends on: 307322
I will retest http://www.carto.net/papers/svg/samples/symbol.svg after bug
307322 is fixed.
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 :)
Depends on: 307444
I will retest everything under
http://www.w3.org/Graphics/SVG/Test/20030813/htmlframe/full-index.html once bug
307444 is fixed.
... and http://www.svgbasics.com/.
Depends on: 307470
I will retest http://www.svgbasics.com/examples/marker_ex1.svg after bug 307470
is fixed.
Depends on: 307565
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.
Attachment #195379 - Attachment is obsolete: true
Blocks: 307826
Depends on: 307839
No longer blocks: 307826
Depends on: 307826
Depends on: 307854
Depends on: 307989
Depends on: 307992
Depends on: 308120
Depends on: 308278
* 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
List includes URLs I found crashes on and URLs that use rarely-used Gecko
features.
Attachment #194596 - Flags: review?(bernd.mielke) → review+
Depends on: 308585
Depends on: 308752
FYI, I'm not authorized to access the majority of the bugs you're referring,
in case you want any help on those...
Depends on: 308917
Depends on: 309120
Depends on: 309122
Depends on: 309732
Depends on: 291531, 291902
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>)
Depends on: 310267
Depends on: 310426
Depends on: 310436
Depends on: 310520
We'll evaluate each crash fix as they come in. No longer blocking on tracking bugs.
Flags: blocking1.8b5+ → blocking1.8b5-
Depends on: 310556
Depends on: 310638
Attached file Stir DOM recorder (obsolete) —
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.
Depends on: 311478
Whiteboard: [sg:investigate]
Blocks: fuzz
Depends on: 317544
Depends on: 317545
Depends on: 317546
Depends on: 317547
Depends on: 317549
Flags: blocking1.8.0.1?
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.
Depends on: 321299
Attached file Stir DOM recorder (obsolete) —
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
Flags: blocking1.8.0.1? → blocking1.8.0.1+
Whiteboard: [sg:investigate] → [sg:nse] meta
Flags: blocking1.8.0.1+
Depends on: 323022
Depends on: 205735
Depends on: 323585
Depends on: 315549
Depends on: 323604
Depends on: 323978
Depends on: 324318
Depends on: 294022
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.
Depends on: 254144
Depends on: 324918
No longer depends on: 254144
Depends on: 325427
Depends on: 325984
Depends on: 328944
Depends on: 328946
Depends on: 329335
Depends on: 314307
Depends on: 329407
Depends on: 329884
Depends on: 329891
Depends on: 330010
Depends on: 330925
Depends on: 330998
Keywords: crash
Depends on: 335896
Depends on: 336065
Depends on: 336074
Depends on: 337066
Depends on: 337412
Depends on: 337419
Depends on: 338251
Depends on: 338301
Depends on: 338312
Depends on: 338649
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.
Attached file Stir DOM 1.8 (requires fuzz.js) (obsolete) —
* 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
Depends on: 340083
Depends on: 340733
Depends on: 342120
Depends on: 342145
Attached file Stir DOM 2.0 (obsolete) —
Updated for fuzz.js 2.0.
Attachment #224049 - Attachment is obsolete: true
Depends on: 342923
Depends on: 342942
Depends on: 342954
Depends on: 343935
Depends on: 344881
Depends on: 344898
Depends on: 346512
Depends on: 347348
Depends on: 347355
Depends on: 347495
Depends on: 348708
Depends on: 348709
Depends on: 349355
Depends on: 350128
Depends on: 351627
Depends on: 351628
Depends on: 322625
Depends on: 354133
Depends on: 354510
Depends on: 356325
Attached file Stir DOM 3.0 (obsolete) —
Attachment #226744 - Attachment is obsolete: true
Depends on: 359371
Depends on: 361226
Depends on: 361389
Shouldn't have security bugs assigned to nobody. Jesse can own his test bugs
Assignee: nobody → jruderman
Depends on: 363448
Depends on: 365923
Depends on: 366203
Depends on: 366493
Depends on: 366537
Depends on: 366564
Depends on: 367111
Depends on: 367149
Summary: Crashes found by Jesse's "Stir DOM" bookmarklet → Bugs found by Jesse's "Stir DOM" bookmarklet
Depends on: 368860
Depends on: 369126
Depends on: 369438
Depends on: 370876
Depends on: 371125
Depends on: 371124
No longer depends on: 371124
Comment on attachment 242967 [details]
Stir DOM 3.0

New version in bug 339948.
Attachment #242967 - Attachment is obsolete: true
Severity: critical → normal
Version: 1.8 Branch → Trunk
Depends on: 372550
Depends on: 374882
Depends on: 375058
Depends on: 376666
Depends on: 377470
Depends on: 377783
Depends on: 379217
Depends on: 379872
Depends on: 380482
Depends on: 380550
Depends on: 381057
Depends on: 381502
Depends on: 382212
Depends on: 382507
Depends on: 383979
Depends on: 384391
Depends on: 384392
Depends on: 384504
Depends on: 384637
Depends on: 385118
Depends on: 385132
Depends on: 385289
Depends on: 386386
Depends on: 386575
Depends on: 386807
Depends on: 386939
Depends on: 387227
Depends on: 388172
Depends on: 389014
Depends on: 389151
Depends on: 389326
Depends on: 389636
Depends on: 390976
Depends on: 392132
Depends on: 393475
Depends on: 393649
Depends on: 393656
Depends on: 393661
Depends on: 393671
Depends on: 393746
Depends on: 393749
Depends on: 393758
Depends on: 393822
Depends on: 394111
Depends on: 395340
Depends on: 396744
Depends on: 397112
Depends on: 397551
Depends on: 397844
Depends on: 397856
Depends on: 398021
Depends on: 399365
Depends on: 399412
Depends on: 399687
Depends on: 399694
Depends on: 399858
Depends on: 400078
Depends on: 400349
No longer depends on: 323978
Depends on: 401393
Depends on: 401395
Depends on: 402172
Depends on: 402400
Depends on: 402408
Depends on: 403134
Depends on: 403369
Depends on: 404213
Depends on: 404215
Depends on: 404470
Depends on: 404721
Depends on: 405639
Depends on: 406902
Depends on: 408292
Depends on: 408904
Depends on: 411835
Depends on: 412243
Depends on: 412543
Depends on: 413079
Depends on: 413085
Depends on: 413174
Depends on: 414719
Depends on: 416107
Depends on: 416734
Depends on: 419527
Depends on: 420031
Depends on: 421393
Depends on: 423107
Depends on: 423264
Depends on: 425981
Depends on: 426040
Depends on: 427325
Depends on: 428113
Depends on: 429454
Depends on: 429881
Depends on: 430991
Depends on: 432058
Depends on: 432752
Depends on: 433450
Depends on: 434894
Depends on: 436204
Depends on: 442860
Depends on: 443528
Depends on: 443538
Depends on: 443655
Depends on: 444484
Depends on: 445288
Depends on: 448064
Depends on: 448993
Depends on: 449129
Depends on: 453406
Depends on: 453736
Depends on: 454361
Depends on: 455614
Depends on: 457514
Depends on: 458493
Depends on: 460209
Depends on: 460323
Depends on: 460910
Depends on: 460924
Depends on: 461289
Depends on: 462968
Depends on: 463741
Depends on: 464589
Depends on: 466585
Depends on: 466763
Depends on: 468578
Depends on: 468773
Depends on: 470063
Depends on: 470167
Depends on: 472237
Depends on: 473042
Depends on: 474041
Depends on: 474377
Depends on: 476579
Depends on: 477878
Depends on: 477928
Depends on: 478527
Depends on: 479938
Depends on: 481089
Depends on: 481139
Depends on: 481806
Depends on: 482398
Depends on: 487539
Depends on: 487724
Depends on: 489480
Depends on: 489501
Depends on: 494225
Depends on: 494332
Depends on: 495546
Depends on: 496011
Depends on: 496062
Depends on: 496420
Depends on: 497734
Depends on: 499841
Depends on: 499857
Depends on: 500847
Depends on: 501878
Depends on: 503699
Depends on: 505399
Depends on: 508154
Depends on: 508919
Depends on: 508908
Depends on: 513153
Depends on: 522516
Depends on: 523468
Depends on: 531550
Depends on: 535632
Depends on: 536720
Depends on: 537624
Depends on: 538062
Depends on: 538210
Depends on: 538233
Depends on: 538267
Depends on: 539167
Depends on: 540760
Depends on: 545574
Depends on: 550355
Depends on: 550362
Depends on: 551620
Depends on: 557348
Depends on: 559491
Depends on: 560447
Depends on: 564063
Depends on: 564368
Depends on: 564968
Depends on: 571105
Depends on: 571981
Depends on: 572003
Depends on: 572607
Depends on: 575446
Depends on: 580129
Depends on: 580140
Depends on: 580151
Depends on: 584208
Depends on: 587336
Depends on: 589316
Depends on: 590395
Depends on: 591141
Depends on: 591409
Depends on: 591480
Depends on: 592129
Depends on: 595783
Depends on: 596796
Depends on: 596876
Depends on: 597240
Depends on: 597317
Depends on: 604843
Depends on: 605340
Depends on: 605345
Depends on: 606101
Depends on: 606430
Depends on: 606642
Depends on: 611927
Depends on: 612736
Depends on: 615002
Depends on: 615944
Depends on: 616748
Depends on: 617089
Depends on: 621598
Depends on: 627647
Depends on: 635442
Depends on: 635636
Depends on: 637214
Depends on: 643853
Depends on: 650489
Depends on: 654928
Depends on: 655451
Depends on: 656130
Depends on: 665334
Depends on: 667025
Depends on: 667321
Depends on: 669225
Depends on: 674223
Depends on: 690979
Depends on: 690990
Depends on: 690994
Depends on: 691096
Depends on: 693142
Depends on: 695573
Depends on: 696175
Depends on: 707098
Depends on: 709536
Depends on: 710149
Depends on: 713413
Depends on: 713499
Depends on: 716503
Depends on: 718236
Depends on: 718290
Depends on: 723382
Depends on: 723657
Depends on: 725918
Depends on: 725928
Depends on: 729431
Depends on: 736389
Depends on: 736924
Depends on: 738555
Depends on: 740199
Depends on: 742190
Depends on: 745699
Depends on: 757751
Depends on: 760957
Depends on: 766471
Depends on: 767233
Depends on: 779707
Depends on: 780764
Depends on: 788831
Depends on: 788929
Depends on: 791430
Depends on: 791601
Depends on: 797945
Depends on: 803562