Last Comment Bug 493281 - Possible Stack Corruption starting at Unknown Symbol @ 0x6d89c0006d89c
: Possible Stack Corruption starting at Unknown Symbol @ 0x6d89c0006d89c
Status: VERIFIED FIXED
[sg:critical?]
: crash, fixed1.8.1.22, testcase, verified1.9.0.12, verified1.9.1
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.9.2a1
Assigned To: Blake Kaplan (:mrbkap)
:
: Andrew Overholt [:overholt]
Mentors:
http://www.tuning.fr/compts/compteurs...
: 493243 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-15 12:56 PDT by Carsten Book [:Tomcat]
Modified: 2016-05-11 15:47 PDT (History)
14 users (show)
jst: blocking1.9.1+
dveditz: blocking1.9.0.12+
dveditz: wanted1.9.0.x+
dveditz: blocking1.8.1.next+
dveditz: wanted1.8.1.x+
mrbkap: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (1.03 KB, patch)
2009-05-15 19:16 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Fix (1.78 KB, patch)
2009-05-15 19:38 PDT, Blake Kaplan (:mrbkap)
jst: review+
jst: superreview+
Details | Diff | Splinter Review
Just the crashtest (983 bytes, patch)
2009-05-18 16:44 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Patch for older branches (1.24 KB, patch)
2009-05-18 17:06 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
mrbkap: superreview+
dveditz: approval1.9.0.12+
dveditz: approval1.8.1.next+
Details | Diff | Splinter Review
crashtest (1.29 KB, patch)
2009-06-01 16:12 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
patch for 1.8.0 (1.51 KB, patch)
2009-07-09 08:16 PDT, Jan Horak
mrbkap: review+
Details | Diff | Splinter Review

Description Carsten Book [:Tomcat] 2009-05-15 12:56:35 PDT
Steps to reproduce:

-> Load http://www.tuning.fr/compts/compteurs.php?compts_id=1670&scrw=1680
--> Crashs 1.9.1 opt/debug builds Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090515 Shiretoko/3.5b5pre and trunk 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090515 Minefield/3.6a1pre

(fa8.18c): Access violation - code c0000005 (!!! second chance !!!)
eax=3ecb3f57 ebx=7ffdf000 ecx=0012ea4c edx=06389f15 esi=00d0b4e8 edi=00150000
eip=06389f39 esp=0012d934 ebp=0012d950 iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000202
06389f39 3b00            cmp     eax,dword ptr [eax]  ds:0023:3ecb3f57=????????


The stack trace contains one or more locations for which no symbol or module could be found. This may be a sign of stack corruption.
ChildEBP RetAddr
WARNING: Frame IP not in any known module. Following frames may be wrong.
0012d930 0028b31d 0x6389f39
0012d950 02c6dc49 xpcom_core!nsQueryInterface::operator()+0x2d
0284f81c 6c6c697a gklayout!nsCOMPtr<nsIDOMHTMLDocument>::assign_from_qi+0x19
0284f81c 00000000 0x6c6c697a
quit:
Comment 1 Daniel Veditz [:dveditz] 2009-05-15 17:26:16 PDT
keeping !exploitable output security sensitive for now. Also "Frame IP not in any known module" doesn't sound good.
Comment 2 Blake Kaplan (:mrbkap) 2009-05-15 19:07:41 PDT
(gdb) p obj
$2 = (JSObject *) 0xac886de0
(gdb) call js_DumpObject ($)
object 0xac886de0
class 0xb7e493e0 With
properties:
slots:
   0 (proto) = <HTML document.all class object at 0xac886ae0>
Comment 3 Blake Kaplan (:mrbkap) 2009-05-15 19:16:03 PDT
Created attachment 377816 [details] [diff] [review]
Fix

I was tempted to elide the if statement entirely, but I suppose it's worth being consistent (and paranoid) in non-performance-sensitive code.
Comment 4 chris hofmann 2009-05-15 19:24:02 PDT
and ye shall be known as the signature and stack

Firefox 3.5b5pre Crash Report 
[@ nsCOMPtr_base::assign_from_qi(nsQueryInterface, nsID const&) ]

0           @0x2c4f218d      
1     XUL     nsCOMPtr_base::assign_from_qi     nsCOMPtr.cpp:96 

http://crash-stats.mozilla.com/report/index/eccf9a30-a815-4b21-a9e4-0d8d62090515?p=1

problem doesn't show up in the top #100 crash list but that may just be related
to the variation in the kind of stacks this crash is producing and the way we generate signatures.
Comment 5 Blake Kaplan (:mrbkap) 2009-05-15 19:31:10 PDT
I don't think this is common -- it requires with (document.all) foo. I'll write up a testcase.
Comment 6 Blake Kaplan (:mrbkap) 2009-05-15 19:38:59 PDT
Created attachment 377819 [details] [diff] [review]
Fix

I took this opportunity to make the null check not vacuous in form (even though it is in practice).
Comment 7 Blake Kaplan (:mrbkap) 2009-05-18 13:01:30 PDT
*** Bug 493243 has been marked as a duplicate of this bug. ***
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2009-05-18 16:16:00 PDT
This sounds highly exploitable, blocking.
Comment 9 Blake Kaplan (:mrbkap) 2009-05-18 16:44:45 PDT
Created attachment 378178 [details] [diff] [review]
Just the crashtest

I don't want to check the testcase in before this is fixed everywhere, so I'm splitting out the crashtest. If someone wants to check this in before me, this should be ready to go.
Comment 10 Blake Kaplan (:mrbkap) 2009-05-18 16:56:35 PDT
http://hg.mozilla.org/mozilla-central/rev/29664b7cdd11
Comment 11 Blake Kaplan (:mrbkap) 2009-05-18 17:06:36 PDT
Created attachment 378190 [details] [diff] [review]
Patch for older branches

dom/base/nsDOMClassInfo.cpp -> dom/src/base/nsDOMClassInfo.cpp.
Comment 12 Blake Kaplan (:mrbkap) 2009-05-19 12:42:49 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/cc1ad4713974
Comment 13 Daniel Veditz [:dveditz] 2009-05-20 15:56:54 PDT
Comment on attachment 378190 [details] [diff] [review]
Patch for older branches

Approved for 1.8.1.22, a=dveditz for release-drivers

The 1.9.0 branch is currently closed for the 1.9.0.11 release, we'll approve this when it opens.
Comment 14 Daniel Veditz [:dveditz] 2009-05-22 11:05:17 PDT
Comment on attachment 378190 [details] [diff] [review]
Patch for older branches

Approved for 1.9.0.12, a=dveditz for release-drivers
Comment 15 Daniel Veditz [:dveditz] 2009-05-31 14:58:38 PDT
Blake: the crashtest in this bug does not crash Firefox 2 -- do we still need it on the 1.8 branch?
Comment 16 Henrik Skupin (:whimboo) 2009-06-01 14:25:07 PDT
Verified fixed on trunk and 1.9.1 with builds on all platforms like:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090601 Minefield/3.6a1pre ID:20090601031227

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090601 Shiretoko/3.5pre ID:20090601031153
Comment 17 Blake Kaplan (:mrbkap) 2009-06-01 16:12:42 PDT
Created attachment 380944 [details] [diff] [review]
crashtest

Only the second crashtest here crashes on the 1.8 branch (they both crash on trunk).
Comment 18 Blake Kaplan (:mrbkap) 2009-06-01 16:19:13 PDT
new revision: 1.292.2.70; previous revision: 1.292.2.69
Comment 19 Henrik Skupin (:whimboo) 2009-06-01 16:28:19 PDT
Blake, when will the tests be checked into 1.9.1 and trunk?
Comment 20 Blake Kaplan (:mrbkap) 2009-06-01 16:30:11 PDT
When this bug is opened up.
Comment 21 Blake Kaplan (:mrbkap) 2009-06-01 16:35:47 PDT
new revision: 1.550; previous revision: 1.549
Comment 22 Al Billings [:abillings] 2009-06-30 12:45:29 PDT
Verified crash in 1.9.0.11 and fix in 1.9.0.12 using referenced original page and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.12pre) Gecko/2009063012 GranParadiso/3.0.12pre.
Comment 23 Jan Horak 2009-07-09 08:16:41 PDT
Created attachment 387643 [details] [diff] [review]
patch for 1.8.0

Added patch for 1.8.0 version. Could you please try to do the review? Thanks a lot!
Comment 24 Blake Kaplan (:mrbkap) 2009-07-09 10:18:41 PDT
Comment on attachment 387643 [details] [diff] [review]
patch for 1.8.0

>diff -upU8 mozilla/dom/src/base/nsDOMClassInfo.cpp.493281 mozilla/dom/src/base/nsDOMClassInfo.cpp
>+#include "jsobj.h"

This won't be needed with my comment below.

>+  while (OBJ_GET_CLASS(cx, obj) != &sHTMLDocumentAllClass) {
>+    obj = OBJ_GET_PROTO(cx, obj);
>+    if (!obj) {
>+      NS_ERROR("The JS engine lies!");
>+      return JS_TRUE;
>+    }
>+  }

On the 1.8 branch, I think it makes sense to s/OBJ_GET_CLASS/JS_GET_CLASS/ and s/OBJ_GET_PROTO/JS_GetPrototype/ since you're not going to funnel directly into the STOBJ_GET_* case that we get on m-c and 1.9.1. r=mrbkap either way, though.
Comment 25 Blake Kaplan (:mrbkap) 2009-08-10 16:36:49 PDT
http://hg.mozilla.org/mozilla-central/rev/505615255899

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