crash in InstallTrigger.install.call

VERIFIED FIXED in mozilla1.8beta2

Status

Core Graveyard
Installer: XPInstall Engine
P1
critical
VERIFIED FIXED
13 years ago
2 years ago

People

(Reporter: georgi - hopefully not receiving bugspam, Assigned: dveditz)

Tracking

(4 keywords)

Trunk
mozilla1.8beta2
x86
All
crash, fixed-aviary1.0.3, fixed1.7.7, testcase
Bug Flags:
blocking1.7.7 +
blocking1.8b2 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix])

Attachments

(4 attachments, 3 obsolete attachments)

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
Created attachment 180577 [details]
testcase
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
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

13 years ago
Keywords: crash

Updated

13 years ago
Keywords: testcase
Created attachment 180580 [details]
win2k3 stack trace from 20050404 (optimized with symbols)
This is core (trying JS Engine)
Assignee: firefox → general
Severity: normal → critical
Component: General → JavaScript Engine
OS: Linux → All
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk

Comment 5

13 years ago
Created attachment 180583 [details]
winxpsp2 stack from debug seamonkey 20050412

completely different stack
(Reporter)

Updated

13 years ago
Group: security
Severity: critical → normal
Component: JavaScript Engine → Build Config
OS: All → Linux
Product: Core → Firefox
Version: Trunk → 1.0 Branch
bugzilla overwrote some entries on reload, feel free to correct.
Component: Build Config → Build Config
OS: Linux → All
Product: Firefox → Core
Version: 1.0 Branch → Trunk

Updated

13 years ago
Component: Build Config → Installer: XPInstall Engine
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
Assignee: general → dveditz
Severity: normal → critical
Flags: blocking1.8b2+
Flags: blocking1.7.7?
Flags: blocking-aviary1.0.3?
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
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
	alert(InstallVersion.toString());
	alert(InstallVersion.toString.call(document,"a","a"));
	
	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
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
> 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

Status: NEW → ASSIGNED
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.
Created attachment 180623 [details] [diff] [review]
fix

Need to test returns from JS_GetInstancePrivate and fail if null.

/be
Attachment #180609 - Attachment is obsolete: true
Attachment #180623 - Flags: superreview?(shaver)
Attachment #180623 - Flags: review?(dveditz)
Attachment #180623 - Flags: approval1.8b2+
Attachment #180623 - Flags: approval1.7.7?
Attachment #180623 - Flags: approval-aviary1.0.3?
Comment on attachment 180623 [details] [diff] [review]
fix

sr=shaver
Attachment #180623 - Flags: superreview?(shaver) → superreview+
Comment on attachment 180623 [details] [diff] [review]
fix

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

/be
Attachment #180623 - Attachment is obsolete: true
Attachment #180623 - Flags: review?(dveditz)
Attachment #180623 - Flags: approval1.8b2+
Attachment #180623 - Flags: approval1.7.7?
Attachment #180623 - Flags: approval-aviary1.0.3?
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
Attachment #180631 - Flags: superreview?(shaver)
Attachment #180631 - Flags: review?(dveditz)
Attachment #180631 - Flags: approval1.8b2+
Attachment #180631 - Flags: approval1.7.7?
Attachment #180631 - Flags: approval-aviary1.0.3?
Comment on attachment 180631 [details] [diff] [review]
fix, v2

r=dveditz
Attachment #180631 - Flags: review?(dveditz) → review+

Comment 19

13 years ago
Comment on attachment 180631 [details] [diff] [review]
fix, v2

a=chofmann for the branches -- assuming shaver's sr carries over
Attachment #180631 - Flags: approval1.7.7?
Attachment #180631 - Flags: approval1.7.7+
Attachment #180631 - Flags: approval-aviary1.0.3?
Attachment #180631 - Flags: approval-aviary1.0.3+
Fixed on both branches and trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Comment on attachment 180631 [details] [diff] [review]
fix, v2

sr=shaver, encore.
Attachment #180631 - Flags: superreview?(shaver) → superreview+
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.
Status: RESOLVED → VERIFIED
Keywords: fixed-aviary1.0.3
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)	
almost sure this is reliably exploitable.
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.
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

13 years ago
Looks like a fix or related fix for this bug caused bug 290312 on the aviary
branch.  See attachment 180703 [details].
Shaver's kindly patching the regression.

/be
oops, didn't see this since the mac doesn't have an installer. I can reopen if
needed.
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.
Flags: blocking1.7.7? → blocking1.7.7+
Blocks: 256195
Whiteboard: [sg:fix]
Created attachment 180797 [details]
please respect comments in the testcase

please respect comments in the testcase
Attachment #180577 - Attachment is obsolete: true
Fix released
Group: security

Updated

12 years ago
Flags: blocking-aviary1.0.3?

Comment 33

12 years ago
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).
Keywords: fixed1.7.7
Blocks: 256197
No longer blocks: 256195

Comment 34

12 years ago
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

12 years ago
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

12 years ago
> 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

12 years ago
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.

Updated

12 years ago
Flags: testcase+

Updated

11 years ago
Flags: in-testsuite+ → in-testsuite?

Comment 38

8 years ago
js/src/xpconnect/crashtests/290162-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3
Flags: in-testsuite? → in-testsuite+
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.