Last Comment Bug 624187 - (CVE-2011-0072) Use after free after appending a frame/iframe element to a DOM tree with the NoScript add-on enabled.
(CVE-2011-0072)
: Use after free after appending a frame/iframe element to a DOM tree with the ...
Status: RESOLVED FIXED
: [sg:critical?][hardblocker] not sure ...
:
Product: Core
Classification: Components
Component: XPConnect
: Trunk
: All All
: -- critical (vote)
: mozilla2.0b10
Assigned To: Mounir Lamouri (:volkmar)
: xpconnect
:
:
:
  Show dependency treegraph
 
Reported: 2011-01-08 13:13 PST by Martin Barbella
Modified: 2011-05-24 23:38 PDT (History)
6 users (show)
See Also:
Crash Signature:
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  betaN+
  ---
  .17+
  .17-fixed
  .19+
  .19-fixed


Attachments
Testcase which triggers the crash (referred to in the initial report as 1.html). (783 bytes, text/html)
2011-01-08 13:15 PST, Martin Barbella
no flags Details
Stack trace and other information from gdb. (7.96 KB, text/plain)
2011-01-08 13:16 PST, Martin Barbella
no flags Details
Output from valgrind (with flags --malloc-fill=aa --free-fill=bb --leak-check=no). (6.50 KB, text/plain)
2011-01-08 13:17 PST, Martin Barbella
no flags Details
Patch v1 (6.71 KB, patch)
2011-01-12 06:46 PST, Mounir Lamouri (:volkmar)
Olli.Pettay: review+
Details | Diff | Splinter Review
Patch v1.1 (7.36 KB, patch)
2011-01-13 08:54 PST, Mounir Lamouri (:volkmar)
Olli.Pettay: review+
dveditz: approval1.9.2.17+
Details | Diff | Splinter Review
Inter diff (845 bytes, patch)
2011-01-13 08:55 PST, Mounir Lamouri (:volkmar)
no flags Details | Diff | Splinter Review
Patch for 1.9.1 (8.88 KB, patch)
2011-01-29 15:16 PST, Mounir Lamouri (:volkmar)
Olli.Pettay: review+
dveditz: approval1.9.1.19+
Details | Diff | Splinter Review

Summon comment box

Description Martin Barbella 2011-01-08 13:13:37 PST
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.
Comment 1 Martin Barbella 2011-01-08 13:15:00 PST
Created attachment 502284 [details]
Testcase which triggers the crash (referred to in the initial report as 1.html).
Comment 2 Martin Barbella 2011-01-08 13:16:01 PST
Created attachment 502286 [details]
Stack trace and other information from gdb.
Comment 3 Martin Barbella 2011-01-08 13:17:10 PST
Created attachment 502288 [details]
Output from valgrind (with flags --malloc-fill=aa --free-fill=bb --leak-check=no).
Comment 4 Andreas Gal :gal 2011-01-08 13:22:08 PST
b? so the drivers see this, we still need confirmation whats affected and whats not
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2011-01-10 17:41:18 PST
Ben, can you have a look at this one?
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2011-01-10 17:44:50 PST
Actually, maybe Mounir could dig in here?
Comment 7 Mounir Lamouri (:volkmar) 2011-01-11 16:10:01 PST
Is there a reason why nsDocShellEnumerator keeps weak reference to nsIDocShellTreeItem?
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2011-01-11 18:13:58 PST
Cc:ing more people who might be able to answer Mounir's question above regarding nsDocShellEnumerator...
Comment 9 Boris Zbarsky (:bz) 2011-01-11 19:02:35 PST
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.
Comment 10 Mounir Lamouri (:volkmar) 2011-01-12 06:46:50 PST
Created attachment 503139 [details] [review]
Patch v1

This is fixing the crash locally.
Comment 11 Daniel Veditz 2011-01-12 10:25:23 PST
Is this really sg:critical? is there any way to cause this crash in a default configuration?
Comment 12 Mounir Lamouri (:volkmar) 2011-01-12 15:58:20 PST
This patch seems to turn some tests orange on the try server.
Comment 13 Mounir Lamouri (:volkmar) 2011-01-13 08:54:52 PST
Created attachment 503508 [details] [review]
Patch v1.1

This should pass the try server.
Comment 14 Mounir Lamouri (:volkmar) 2011-01-13 08:55:23 PST
Created attachment 503509 [details] [review]
Inter diff
Comment 15 Daniel Veditz 2011-01-13 13:35:39 PST
(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?]
Comment 16 Mounir Lamouri (:volkmar) 2011-01-14 07:55:00 PST
Pushed:
http://hg.mozilla.org/mozilla-central/rev/e23b4fe4e09c
Comment 17 Mounir Lamouri (:volkmar) 2011-01-14 07:56:13 PST
For 1.9.2, the same patch will be usable.
For 1.9.1, it will require more work.
Comment 19 Mounir Lamouri (:volkmar) 2011-01-29 15:16:28 PST
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 20 Daniel Veditz 2011-01-31 10:42:46 PST
Comment on attachment 503508 [details] [review]
Patch v1.1

approved for 1.9.2.15, a=dveditz for release-drivers
Comment 21 Daniel Veditz 2011-01-31 10:43:37 PST
Comment on attachment 508166 [details] [review]
Patch for 1.9.1

approved for 1.9.1.18, a=dveditz for release-drivers
Comment 23 Daniel Veditz 2011-03-04 10:15:03 PST
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.

Note You need to log in before you can comment on or make changes to this bug.