Last Comment Bug 290162 - crash in InstallTrigger.install.call
: crash in InstallTrigger.install.call
Status: VERIFIED FIXED
[sg:fix]
: crash, fixed-aviary1.0.3, fixed1.7.7, testcase
Product: Core Graveyard
Classification: Graveyard
Component: Installer: XPInstall Engine (show other bugs)
: Trunk
: x86 All
: P1 critical (vote)
: mozilla1.8beta2
Assigned To: Daniel Veditz [:dveditz]
:
Mentors:
Depends on:
Blocks: sbb+
  Show dependency treegraph
 
Reported: 2005-04-13 06:16 PDT by georgi - hopefully not receiving bugspam
Modified: 2015-12-11 07:21 PST (History)
10 users (show)
dveditz: blocking1.7.7+
brendan: blocking1.8b2+
bob: in‑testsuite+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (204 bytes, text/html)
2005-04-13 06:17 PDT, georgi - hopefully not receiving bugspam
no flags Details
win2k3 stack trace from 20050404 (optimized with symbols) (5.21 KB, text/plain)
2005-04-13 06:39 PDT, Matthias Versen [:Matti]
no flags Details
winxpsp2 stack from debug seamonkey 20050412 (3.56 KB, text/plain)
2005-04-13 07:18 PDT, Bob Clary [:bc:]
no flags Details
WIP that does what brendan talked about (61.56 KB, patch)
2005-04-13 12:54 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
fix (64.06 KB, patch)
2005-04-13 14:09 PDT, Brendan Eich [:brendan]
shaver: superreview+
Details | Diff | Review
fix, v2 (79.54 KB, patch)
2005-04-13 14:54 PDT, Brendan Eich [:brendan]
dveditz: review+
shaver: superreview+
chofmann: approval‑aviary1.0.3+
chofmann: approval1.7.7+
brendan: approval1.8b2+
Details | Diff | Review
please respect comments in the testcase (279 bytes, text/html)
2005-04-15 07:44 PDT, georgi - hopefully not receiving bugspam
no flags Details

Description georgi - hopefully not receiving bugspam 2005-04-13 06:16:25 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050408 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050408 Firefox/1.0.3

InstallTrigger.install.call(document,"a","a");
crashes in a strange way, jumping to strange places.
crashes at different $eip which shows it may be exploitable.

last crash:

0x08b9e161 in ?? ()
(gdb) x/i $eip
0x8b9e161:      in     $0xaf,%eax
(gdb) 


Reproducible: Always

Steps to Reproduce:
will attach a testacase
Comment 1 georgi - hopefully not receiving bugspam 2005-04-13 06:17:41 PDT
Created attachment 180577 [details]
testcase
Comment 2 Steve England [:stevee] 2005-04-13 06:24:41 PDT
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050413
Firefox/1.0+

I also crash out with the testcase. TBID TB5058239M
Comment 3 Matthias Versen [:Matti] 2005-04-13 06:39:55 PDT
Created attachment 180580 [details]
win2k3 stack trace from 20050404 (optimized with symbols)
Comment 4 Matthias Versen [:Matti] 2005-04-13 06:41:41 PDT
This is core (trying JS Engine)
Comment 5 Bob Clary [:bc:] 2005-04-13 07:18:17 PDT
Created attachment 180583 [details]
winxpsp2 stack from debug seamonkey 20050412

completely different stack
Comment 6 georgi - hopefully not receiving bugspam 2005-04-13 08:54:57 PDT
bugzilla overwrote some entries on reload, feel free to correct.
Comment 7 Brendan Eich [:brendan] 2005-04-13 09:10:40 PDT
http://lxr.mozilla.org/mozilla/source/xpinstall/src/nsJSInstallTriggerGlobal.cpp#158

nsJSInstallTriggerGlobal.cpp needs to be scoured of all JS_GetPrivate calls in
methods that can be invoked on unrelated |this| object types.  The right API has
always been JS_GetInstancePrivate.  Not sure how exploitable this is, but it's
an easy bug to fix.

/be
Comment 8 Brendan Eich [:brendan] 2005-04-13 09:24:51 PDT
Seven JS_GetPrivate calls in nsJSInstallTriggerGlobal.cpp.  Six of them need to
be changed to JS_GetInstancePrivate.  The call from the finalizer is fine, it
should not be changed.

Any other ancient 4.x-era hand-rolled JS API code lying around?

/be
Comment 9 georgi - hopefully not receiving bugspam 2005-04-13 09:45:34 PDT
	alert(InstallVersion.toString());
	alert(InstallVersion.toString.call(document,"a","a"));
	
Comment 10 georgi - hopefully not receiving bugspam 2005-04-13 09:52:02 PDT
	alert(InstallVersion.toString.call(0xbabe,"a","a"));

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1086057216 (LWP 14155)]
0x4010fbe1 in nsSubstring::Replace () from ./local/firefox103/libxpcom.so

(gdb) x/i $eip
0x4010fbe1 <_ZN11nsSubstring7ReplaceEjjPKtj+281>:       cmpw   $0x0,(%eax)
(gdb) p $eax
$4 = 95612
Comment 11 Brendan Eich [:brendan] 2005-04-13 10:09:47 PDT
http://lxr.mozilla.org/mozilla/search?string=JS_GetPrivate%28

Look at all xpinstall uses.  Any from a JS native function (a function with the
JSNative signature:

JSBool
(*JSNative)(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval)

should be changed to call JS_GetInstancePrivate, passing argv as the final param.

/be
Comment 12 Brendan Eich [:brendan] 2005-04-13 10:17:56 PDT
> should be changed to call JS_GetInstancePrivate, passing argv as the final
> param.

And null-test that API's return value, and return JS_FALSE if null.

/be

Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2005-04-13 12:54:30 PDT
Created attachment 180609 [details] [diff] [review]
WIP that does what brendan talked about

This should take care of the JS_GetPrivate()'s mentioned above, but it doesn't
fix the crash, at least not completely. With this patch, I now crash at:

#0  0x00002aaaadd878fa in XPCWrappedNativeScope::GetGlobalJSObject (
    this=0x100000009) at xpcprivate.h:1000
#1  0x00002aaaaddc972b in IsSystemCallingContent (cx=0x782be0,
    wrapper_or_proto={{mMaybeWrapper = 0xe59c08, mMaybeProto = 0xe59c08}})
    at
../../../../../mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:947
#2  0x00002aaaaddca4e1 in XPC_WN_JSOp_Safe_GetProperty (cx=0x782be0,
    obj=0x897ab0, id=8632352, vp=0x7fffffffe238)
    at
../../../../../mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1282
#3  0x00002aaaaab144d0 in js_Interpret (cx=0x782be0, result=0x7fffffffe488)
    at ../../../mozilla/js/src/jsinterp.c:2823
#4  0x00002aaaaab0653d in js_Invoke (cx=0x782be0, argc=2, flags=2)
    at ../../../mozilla/js/src/jsinterp.c:966
#5  0x00002aaaaab06980 in js_InternalInvoke (cx=0x782be0, obj=0x880170,
    fval=11542224, flags=0, argc=2, argv=0xdc0a40, rval=0x7fffffffe7c8)
    at ../../../mozilla/js/src/jsinterp.c:1043

where it looks like we've got a bad wrapper in wrapper_or_proto. I don't have
tiem to continue investigating this right now, so someone else will need to
continue from here.
Comment 14 Brendan Eich [:brendan] 2005-04-13 14:09:52 PDT
Created attachment 180623 [details] [diff] [review]
fix

Need to test returns from JS_GetInstancePrivate and fail if null.

/be
Comment 15 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-04-13 14:41:24 PDT
Comment on attachment 180623 [details] [diff] [review]
fix

sr=shaver
Comment 16 Brendan Eich [:brendan] 2005-04-13 14:46:22 PDT
Comment on attachment 180623 [details] [diff] [review]
fix

I was building on Linux, didn't catch the winreg copy/paste bug.

/be
Comment 17 Brendan Eich [:brendan] 2005-04-13 14:54:18 PDT
Created attachment 180631 [details] [diff] [review]
fix, v2

This is better.  Thanks to dveditz for pointing out an existing, now-wrong and
redundant, null test everywhere; he also pointed out a Windows-only botch or
two.

/be
Comment 18 Daniel Veditz [:dveditz] 2005-04-13 15:00:40 PDT
Comment on attachment 180631 [details] [diff] [review]
fix, v2

r=dveditz
Comment 19 chris hofmann 2005-04-13 15:06:49 PDT
Comment on attachment 180631 [details] [diff] [review]
fix, v2

a=chofmann for the branches -- assuming shaver's sr carries over
Comment 20 Brendan Eich [:brendan] 2005-04-13 15:23:19 PDT
Fixed on both branches and trunk.

/be
Comment 21 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-04-13 15:37:54 PDT
Comment on attachment 180631 [details] [diff] [review]
fix, v2

sr=shaver, encore.
Comment 22 sairuh (rarely reading bugmail) 2005-04-13 21:11:59 PDT
vrfy'd fixed on the branch: testcase no longer crashes --tested with
2005041320-1.0.3 ffox build on mac os x 10.3.8.
Comment 23 georgi - hopefully not receiving bugspam 2005-04-14 00:52:09 PDT
preliminary blackbox approach shows js cares (at least before brendan's patch)
about raw C pointers, which is a sign of exploitability.

the following reads memory:

	alert(InstallVersion.toString.call(0x20087df0,"a","a").toSource());
	// reads memory around 2*0xNUMBER (below)	
Comment 24 georgi - hopefully not receiving bugspam 2005-04-14 01:10:09 PDT
almost sure this is reliably exploitable.
Comment 25 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-04-14 01:29:55 PDT
Yes, that's consistent with the misuse of JS_GetPrivate that brendan patched:
the integer is tagged as a jsval (x << 1 + 1) and then stored in the private
slot of the Number object created to serve as the |this| for the invocation of
InstallTrigger.install.  The pre-patch code will use that jsval as an nsInstall
*, and then manipulate it in whatever interesting ways.  We're now safe from
that, as the Number class will not pass the JS_GetInstancePrivate class-pointer
test and will simply return NULL.

Should probably audit the rest of the tree for such bold uses of JS_GetPrivate
elsewhere, too; lxr reports some things that I would look more closely at if it
weren't 4:30AM.
Comment 26 georgi - hopefully not receiving bugspam 2005-04-14 01:36:08 PDT
i believe grep -rniI STRING * in the tree is better than lxr - lxr may have
missed something because of a bug.

or grep with other options.
Comment 27 Chase Phillips 2005-04-14 07:48:21 PDT
Looks like a fix or related fix for this bug caused bug 290312 on the aviary
branch.  See attachment 180703 [details].
Comment 28 Brendan Eich [:brendan] 2005-04-14 08:43:23 PDT
Shaver's kindly patching the regression.

/be
Comment 29 sairuh (rarely reading bugmail) 2005-04-14 09:29:34 PDT
oops, didn't see this since the mac doesn't have an installer. I can reopen if
needed.
Comment 30 sairuh (rarely reading bugmail) 2005-04-14 14:08:44 PDT
the testcase in attachment 180577 [details] no longer causes a crash: tested with
2005041412-1.0.3 (linux) and 2005041413-1.0.3 (windowsXP) ffox builds.
Comment 31 georgi - hopefully not receiving bugspam 2005-04-15 07:44:02 PDT
Created attachment 180797 [details]
please respect comments in the testcase

please respect comments in the testcase
Comment 32 Daniel Veditz [:dveditz] 2005-04-15 19:55:48 PDT
Fix released
Comment 33 Juha-Matti Laurio 2005-04-22 14:05:19 PDT
This is Secunia's SA14938's vulnerability #7;
http://secunia.com/advisories/14938/  and
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2005-1159 (CAN-2005-1159).
Comment 34 Pete Collins (MDG) 2005-07-26 20:04:04 PDT
Doug, I presume it was this patch that caused the regression ...

https://bugzilla.mozilla.org/attachment.cgi?id=180631

Looks like a lot of added checks for js contexts in there. I will have a look
tomorrow.
Comment 35 Ginn Chen 2006-01-04 00:38:19 PST
Firefox 1.0.7 crashed when open the last testcase (https://bugzilla.mozilla.org/attachment.cgi?id=180797)
Firefox 1.0.6 won't crash. (Tested on Linux and Solaris)
Mozilla 1.7.12 also crashed.

Firefox 1.5 on Linux doesn't crash, but Firefox 1.5 on Windows crashed.

Reopen this bug?
Comment 36 Ginn Chen 2006-01-05 01:32:57 PST
> Firefox 1.5 on Linux doesn't crash, but Firefox 1.5 on Windows crashed.

Sorry, I was using Firefox 1.0.7 on Windows.
Firefox 1.5 doesn't crash on either Linux or Windows.
Firefox 1.0.7 crashed on every platform.
See TB13556108H
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=13556108
Comment 37 Ginn Chen 2006-01-06 02:39:43 PST
I think I got the bug number which fixed the regression in 1.8 branch and trunk.
That bug is still security-sensitive.
I'm not authorized to access.
Comment 38 Bob Clary [:bc:] 2009-05-09 10:48:36 PDT
js/src/xpconnect/crashtests/290162-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3

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