Closed Bug 493281 Opened 15 years ago Closed 15 years ago

Possible Stack Corruption starting at Unknown Symbol @ 0x6d89c0006d89c

Categories

(Core :: DOM: Core & HTML, defect, P1)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.2a1

People

(Reporter: cbook, Assigned: mrbkap)

References

(
URL
)

Details

(5 keywords, Whiteboard: [sg:critical?])

Attachments

(4 files, 2 obsolete files)

Fix
1.78 KB, patch
jst
: review+
jst
: superreview+
Details | Diff | Splinter Review
Patch for older branches
1.24 KB, patch
mrbkap
: review+
mrbkap
: superreview+
dveditz
: approval1.9.0.12+
dveditz
: approval1.8.1.next+
Details | Diff | Splinter Review
crashtest
1.29 KB, patch
Details | Diff | Splinter Review
patch for 1.8.0
1.51 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review 
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:
Flags: blocking1.9.2?
Flags: blocking1.9.1?
Severity: normal → critical
keeping !exploitable output security sensitive for now. Also "Frame IP not in any known module" doesn't sound good.
Group: core-security
(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>
Component: General → DOM
QA Contact: general → general
Attached patch Fix (obsolete) — — Splinter Review 
I was tempted to elide the if statement entirely, but I suppose it's worth being consistent (and paranoid) in non-performance-sensitive code.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #377816 - Flags: superreview?(jst)
Attachment #377816 - Flags: review?(jst)
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.
I don't think this is common -- it requires with (document.all) foo. I'll write up a testcase.
Attached patch Fix — — Splinter Review 
I took this opportunity to make the null check not vacuous in form (even though it is in practice).
Attachment #377816 - Attachment is obsolete: true
Attachment #377819 - Flags: superreview?(jst)
Attachment #377819 - Flags: review?(jst)
Attachment #377816 - Flags: superreview?(jst)
Attachment #377816 - Flags: review?(jst)
Attachment #377819 - Flags: superreview?(jst)
Attachment #377819 - Flags: superreview+
Attachment #377819 - Flags: review?(jst)
Attachment #377819 - Flags: review+
This sounds highly exploitable, blocking.
Flags: blocking1.9.2?
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1
Attached patch Just the crashtest (obsolete) — — Splinter Review 
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.
http://hg.mozilla.org/mozilla-central/rev/29664b7cdd11
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
dom/base/nsDOMClassInfo.cpp -> dom/src/base/nsDOMClassInfo.cpp.
Attachment #378190 - Flags: superreview+
Attachment #378190 - Flags: review+
Attachment #378190 - Flags: approval1.9.0.12?
Flags: blocking1.8.1.next?
Flags: blocking1.8.0.next?
Attachment #378190 - Flags: approval1.8.1.next?
Attachment #378190 - Flags: approval1.8.0.next?
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12?
Flags: blocking1.9.0.12+
Flags: blocking1.8.1.next?
Flags: blocking1.8.1.next+
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Attachment #378190 - Flags: approval1.8.1.next? → approval1.8.1.next+
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.
Keywords: testcase-wanted
Whiteboard: [sg:critical?]
Attachment #378190 - Flags: approval1.9.0.12? → approval1.9.0.12+
Comment on attachment 378190 [details] [diff] [review]
Patch for older branches

Approved for 1.9.0.12, a=dveditz for release-drivers
Blake: the crashtest in this bug does not crash Firefox 2 -- do we still need it on the 1.8 branch?
Flags: in-testsuite?
Keywords: testcase-wanted → testcase
Target Milestone: mozilla1.9.1 → mozilla1.9.2a1
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
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
OS: Windows XP → All
Hardware: x86 → All
Attached patch crashtest — — Splinter Review 
Only the second crashtest here crashes on the 1.8 branch (they both crash on trunk).
Attachment #378178 - Attachment is obsolete: true
new revision: 1.292.2.70; previous revision: 1.292.2.69
Keywords: fixed1.8.1.22
Blake, when will the tests be checked into 1.9.1 and trunk?
When this bug is opened up.
new revision: 1.550; previous revision: 1.549
Keywords: fixed1.9.0.12
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.
Keywords: fixed1.9.0.12 → verified1.9.0.12
Attached patch patch for 1.8.0 — — Splinter Review 
Added patch for 1.8.0 version. Could you please try to do the review? Thanks a lot!
Attachment #387643 - Flags: review?(mrbkap)
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.
Attachment #387643 - Flags: review?(mrbkap) → review+
Group: core-security
http://hg.mozilla.org/mozilla-central/rev/505615255899
Flags: in-testsuite? → in-testsuite+
Flags: blocking1.8.0.next?
Attachment #378190 - Flags: approval1.8.0.next?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: