Last Comment Bug 738985 - (CVE-2012-0469) heap-use-after-free at mozilla::dom::indexedDB::IDBKeyRange::cycleCollection::Trace
(CVE-2012-0469)
: heap-use-after-free at mozilla::dom::indexedDB::IDBKeyRange::cycleCollection:...
Status: VERIFIED FIXED
[sg:critical][qa+:ashughes][asan]
: regression
Product: Core
Classification: Components
Component: DOM: IndexedDB (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla14
Assigned To: Kyle Huey [:khuey] (khuey@mozilla.com)
:
Mentors:
: 739054 745679 (view as bug list)
Depends on:
Blocks: 692669
  Show dependency treegraph
 
Reported: 2012-03-24 13:21 PDT by Aki Helin
Modified: 2014-06-27 14:23 PDT (History)
14 users (show)
rforbes: sec‑bounty+
dveditz: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
verified
+
verified
verified
12+
verified


Attachments
file to trigger the issue (70 bytes, text/plain)
2012-03-24 13:21 PDT, Aki Helin
no flags Details
asan trace (5.04 KB, text/plain)
2012-03-24 13:23 PDT, Aki Helin
no flags Details
Patch (2.29 KB, patch)
2012-03-24 18:23 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
bent.mozilla: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑esr10+
Details | Diff | Review

Description Aki Helin 2012-03-24 13:21:36 PDT
Created attachment 609037 [details]
file to trigger the issue

ASan reports a heap-use-after-free error when the attached page is opened. The file was minimized against 14.0a1 (2012-03-24) and doesn't seem to reproduce anymore on 12.0, but similar files caused also it to crash during minimization. The trace was https://crash-stats.mozilla.com/report/index/bp-e750dc35-be93-426f-aff3-cd82b2120324

To reproduce, open idb.html in an ASan build and wait about 7 seconds.
Comment 1 Aki Helin 2012-03-24 13:23:12 PDT
Created attachment 609038 [details]
asan trace
Comment 2 Aki Helin 2012-03-24 13:40:04 PDT
This one seems to usually crash Firefox 12.0 when closing the tab:
  <script>
  for (var foo = 0; foo < 1000; foo++) {
     var x = new Array(1000);
     IDBKeyRange.only(1);
     IDBKeyRange.only("'a'").lower;
  }
  </script>

Crash report: https://crash-stats.mozilla.com/report/index/bp-8684865e-5e04-46ed-997a-f0bdf2120324
Comment 3 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-03-24 18:04:14 PDT
I can reproduce this on trunk on Windows.
Comment 4 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-03-24 18:23:35 PDT
Created attachment 609053 [details] [diff] [review]
Patch

IDBKeyRange assumes that it will be unlinked before it is destroyed, but that assumption is wrong, because it is not wrapper cached.  In Aki's testcase the dead IDBKeyRange remains in the XPConnect hashtable because it never called NS_DROP_JS_OBJECTS.

This is a regression from Bug 692669.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-03-24 18:25:15 PDT
Thanks for the bug report and testcase Aki!
Comment 6 Aki Helin 2012-03-25 00:36:33 PDT
Patch looks good here. Neither the attached cases nor the originals cause any issues after applying it.

5h from bug to patch during weekend. Not bad :)
Comment 7 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-03-25 08:35:20 PDT
*** Bug 739054 has been marked as a duplicate of this bug. ***
Comment 8 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-03-26 15:16:55 PDT
Comment on attachment 609053 [details] [diff] [review]
Patch

Review of attachment 609053 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this change:

::: dom/indexedDB/IDBKeyRange.cpp
@@ +351,5 @@
> +IDBKeyRange::~IDBKeyRange()
> +{
> +  if (mRooted) {
> +    NS_DROP_JS_OBJECTS(this, IDBKeyRange);
> +    mCachedLowerVal = JSVAL_VOID;

No need to do anything else after dropping. You're in the destructor.
Comment 9 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-03-28 07:43:40 PDT
I'm a little worried that it is easy to figure out the bug from the fix, so I snuck this one into the tree in a merge changeset.

https://hg.mozilla.org/mozilla-central/rev/c3fd0768d46a
Comment 10 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-03-28 07:46:02 PDT
Comment on attachment 609053 [details] [diff] [review]
Patch

We'll want this everywhere.

[Approval Request Comment]
Regression caused by (bug #): bug 692669
User impact if declined: sg:crit
Testing completed (on m-c, etc.): It is on m-c
Risk to taking this patch (and alternatives if risky): Very low
String changes made by this patch: N/A
Comment 11 Daniel Veditz [:dveditz] 2012-03-28 14:32:33 PDT
We should land this on branches all at once near the end of the Firefox 12 cycle. Code freeze is April 13 (I think) so sometime in the day or two before that.
Comment 12 Alex Keybl [:akeybl] 2012-03-28 15:01:14 PDT
Holding on approval to give this fix some time to bake on m-c.

(In reply to Daniel Veditz [:dveditz] from comment #11)
> We should land this on branches all at once near the end of the Firefox 12
> cycle. Code freeze is April 13 (I think) so sometime in the day or two
> before that.

April 13th is the code freeze. Is this a change in process you'd like to make or is this bug particularly nasty?
Comment 13 Alex Keybl [:akeybl] 2012-04-03 11:59:16 PDT
Comment on attachment 609053 [details] [diff] [review]
Patch

[Triage Comment]
Approving for landing on all branches.
Comment 14 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-04-03 12:02:31 PDT
Did we decide when to land this?
Comment 15 Alex Keybl [:akeybl] 2012-04-03 14:29:11 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #14)
> Did we decide when to land this?

Before beta 5's go-to-build please (4/10).
Comment 17 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-04-17 07:21:13 PDT
How does one get an ASan build for "releases" (10.0.4ESR, 12 Beta, etc)?
Comment 18 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-04-17 14:40:24 PDT
You don't need an ASAN build, just load the testcase in the browser and refresh it a few times.
Comment 19 [On PTO until 6/29] 2012-04-17 15:36:27 PDT
Verified in nightly (Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:14.0) Gecko/20120417 Firefox/14.0a1) using attached testcase per comment 18. Crashes in Firefox 11 on same machine.
Comment 20 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-04-18 08:38:28 PDT
Verified fixed in Firefox ESR 10.0.4pre 2012-04-18.
Comment 21 Daniel Veditz [:dveditz] 2012-04-18 21:26:20 PDT
*** Bug 745679 has been marked as a duplicate of this bug. ***
Comment 23 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-22 15:53:36 PDT
Verified fixed in Firefox 13.0b4 and Firefox 14.0a2 2012-05-22.
Comment 25 Raymond Forbes[:rforbes] 2013-07-19 17:34:57 PDT
rforbes-bugspam-for-setting-that-bounty-flag-20130719

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