Bugzilla@Mozilla – Bug 624187
Use after free after appending a frame/iframe element to a DOM tree with the NoScript add-on enabled.
Last modified: 2011-05-24 23:38:12 PDT
Summon comment box
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101206 Ubuntu/10.04 (lucid) Firefox/3.6.13 Build Identifier: Mozilla Firefox 3.6.13 After appending a frame with a src of "#" to a title element with the NoScript add-on enabled, Firefox will usually crash. This is caused by a read of freed memory. The crash is usually occurring in nsDocShellEnumerator::GetNext(nsISupports**). I have tested this in Firefox 3.6.13 with NoScript 2.0.9.3 on Ubuntu 10.04 (64 bit) and Windows Vista (32 bit). Reproducible: Sometimes Steps to Reproduce: 1. Open the attached testcase, 1.html, in Firefox with the NoScript add-on enabled. 2. If a crash does not occur immediately, either refresh the page a few times, or close the browser. The crash after closing is extremely consistent, but refreshing the page a few times seems to do the trick. Actual Results: The browser will either crash after loading the page, or at exit. Expected Results: The particular script should be executed as expected, and the browser should not crash. The html file that produces the crash, a stack trace using gdb, and output from valgrind will be attached after this bug is created. If I discover any other useful information about this bug, I will include it.
Created attachment 502284 [details] Testcase which triggers the crash (referred to in the initial report as 1.html).
Created attachment 502286 [details] Stack trace and other information from gdb.
Created attachment 502288 [details] Output from valgrind (with flags --malloc-fill=aa --free-fill=bb --leak-check=no).
b? so the drivers see this, we still need confirmation whats affected and whats not
Ben, can you have a look at this one?
Actually, maybe Mounir could dig in here?
Is there a reason why nsDocShellEnumerator keeps weak reference to nsIDocShellTreeItem?
Cc:ing more people who might be able to answer Mounir's question above regarding nsDocShellEnumerator...
Uh.... no. No reason. And if fact, holding raw weak refs there is clearly unsafe. The safe fix here (in terms of not leaking, etc) is probably to use an array of nsWeakPtrs. The next-safer thing is to just make these strong refs and hope. Note that docshell is not CCed.
Created attachment 503139 [details] [review] Patch v1 This is fixing the crash locally.
Is this really sg:critical? is there any way to cause this crash in a default configuration?
This patch seems to turn some tests orange on the try server.
Created attachment 503508 [details] [review] Patch v1.1 This should pass the try server.
Created attachment 503509 [details] [review] Inter diff
(In reply to comment #11) > Is this really sg:critical? is there any way to cause this crash in a default > configuration? Possibly -- we use nsDocShellEnumerators in our own code (as do several add-ons in addition to NoScript) and could maybe get into this state. Assuming the worst back to [sg:critical?]
Pushed: http://hg.mozilla.org/mozilla-central/rev/e23b4fe4e09c
For 1.9.2, the same patch will be usable. For 1.9.1, it will require more work.
Created attachment 508166 [details] [review] Patch for 1.9.1 I had to apply a part of https://hg.mozilla.org/mozilla-central/rev/f2f8545e079b Re-asking a review to double-check.
Comment on attachment 503508 [details] [review] Patch v1.1 approved for 1.9.2.15, a=dveditz for release-drivers
Comment on attachment 508166 [details] [review] Patch for 1.9.1 approved for 1.9.1.18, a=dveditz for release-drivers
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a47c1882ad1d http://hg.mozilla.org/releases/mozilla-1.9.2/rev/04c4b77a3b8f
The "3.6.15" we're releasing today does not fix this bug, the release containing this bug fix has been renamed to "3.6.16" and the bugzilla flags will be updated to reflect that soon. Today's release is a re-release of 3.6.14 plus a fix for a bug that prevented many Java applets from starting up.