The default bug view has changed. See this FAQ.
Bug 598669 (CVE-2010-3183)

LookupGetterOrSetter Remote Code Execution Vulnerability (ZDI-CAN-929)

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: bsterne, Assigned: mrbkap)

Tracking

({crash, testcase})

1.9.2 Branch
crash, testcase
Points:
---

Firefox Tracking Flags

(blocking1.9.2 .11+, status1.9.2 .11-fixed, blocking1.9.1 .14+, status1.9.1 .14-fixed)

Details

(Whiteboard: [sg:critical] fixed on trunk by 509075)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Created attachment 477535 [details]
testcase (crashes 1.9.2)

Submitted to security@m.o today via ZDI:
------

-- ABSTRACT ------------------------------------------------------------

TippingPoint has identified a vulnerability affecting the following  products:

    Mozilla Firefox 3.6.x

-- VULNERABILITY DETAILS -----------------------------------------------

This vulnerability allows remote attackers to execute arbitrary code on vulnerable installations of Mozilla Firefox. User interaction is required to exploit this vulnerability in that the target must visit a malicious page or open a malicious file.

The specific flaw exists in js3250.dll. When a JavaObject is created and deleted, there is not a proper sanity check for the LookupGetterorSetter() function, which can result in a dangling pointer being passed to the JS_ValueToId() function. A remote attacker can exploit this vulnerability to execute arbitrary code under the context of the SYSTEM user.

Version(s)  tested: Mozilla Firefox 3.6.8
Platform(s) tested: Windows XP SP3 x86

From js/src/xpconnect/src/xpcquickstubs.cpp:

JSBool
xpc_qsDefineQuickStubs(JSContext *cx, JSObject *proto, uintN flags,
PRUint32 ifacec, const nsIID **interfaces,
PRUint32 tableSize, const xpc_qsHashEntry *table) {
...
static JSFunctionSpec getterfns[] = {
JS_FN("__lookupGetter__", SharedLookupGetter, 1, 0),
JS_FN("__lookupSetter__", SharedLookupSetter, 1, 0),
JS_FN("__defineGetter__", SharedDefineGetter, 2, 0),
JS_FN("__defineSetter__", SharedDefineSetter, 2, 0),
JS_FS_END
};

if(definedProperty && !JS_DefineFunctions(cx, proto, getterfns))
return JS_FALSE;

return JS_TRUE;
}

static JSBool
SharedLookupGetter(JSContext *cx, uintN argc, jsval *vp) {
return LookupGetterOrSetter(cx, PR_TRUE, vp); }

static JSBool
LookupGetterOrSetter(JSContext *cx, JSBool wantGetter, jsval *vp) {
...
JSObject *obj = JS_THIS_OBJECT(cx, vp);
if (!obj)
return JS_FALSE;
jsval idval = JS_ARGV(cx, vp)[0];

const char *name = JSVAL_IS_STRING(idval)
? JS_GetStringBytes(JSVAL_TO_STRING(idval))
: nsnull;
if(!JS_ValueToId(cx, idval, &interned_id) ||
!JS_LookupPropertyWithFlagsById(cx, obj, interned_id,
JSRESOLVE_QUALIFIED, &obj2, &v) ||
(obj2 &&
!JS_GetPropertyAttrsGetterAndSetterById(cx, obj2, interned_id, &attrs,
&found, &getter, &setter)))
return JS_FALSE;
...
}

Function LookupGetterOrSetter() lacks important sanity check: it assumes that there was at least one argument passed from caller. When __lookupGetter__ is called with no arguments on JavaScript stack, JS_ARGV(cx, vp)[0] returns jsval which represents potentially uninitialized memory (or some leftovers from previous computations?). This, for example, can be a pointer to already freed JS object. Such dangling pointer is later passed to JS_ValueToId().


<<<<<<EDX gets the value 0c0c0c0c, which will soon be called>>>>>>>

0:000> p
eax=0012cb58 ebx=02deb800 ecx=0012cbf0 edx=08000001 esi=0012cbd0
edi=0012cbf0
eip=0031cc2b esp=0012caf0 ebp=0012cb4c iopl=0 nv up ei pl nz na pe nc
cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000206
js3250!JS_ComputeThis+0x8eb:
0031cc2b 50 push eax
0:000> dc edx
08000001 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000011 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000021 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000031 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000041 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000051 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000061 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................
08000071 0c0c0c0c 0c0c0c0c 0c0c0c0c 0c0c0c0c ................


<<<<<<The function that is responsible for calling into EDX>>>>>>

sub_31CC20 proc near

arg_0= dword ptr 4
arg_4= dword ptr 8
arg_8= dword ptr 0Ch

mov eax, [ecx] ; 00312494
mov edx, [eax]
mov eax, [esp+arg_8]
mov edx, [edx+20h]
push eax
mov eax, [esp+4+arg_4]
push eax
push ecx
mov ecx, [esp+0Ch+arg_0]
push ecx
call edx ; calls the value in edx
; in this case edx = 0c0c0c0c

-- CREDIT --------------------------------------------------------------

This vulnerability was discovered by:
    * regenrecht

Updated

7 years ago
Assignee: general → gal

Comment 1

7 years ago
Fixed by bug 509075 I believe. Deferring to mrbkap and jorendorff for final confirmation.
Alias: ZDI-CAN-929
blocking1.9.2: --- → ?
status1.9.2: --- → ?
Keywords: crash, testcase
Summary: LookupGetterOrSetter Remote Code Execution Vulnerability → LookupGetterOrSetter Remote Code Execution Vulnerability (ZDI-CAN-929)
We should not assume this is "it". There's never only one cockroach.

bhackett says sixgill can find any more statically -- cc'ing him to confirm based on details.

/be

Comment 3

7 years ago
Would be cool to write a static analysis that can find the bug in the old code and then see what else it flags. I like it.
I should have reports from sixgill on read overflows of 64-bit quantities (to catch both jsval[] and js::Value[]) tomorrow or Friday.  The tool is (nearly) sound so will definitely find this bug, the challenge is getting it to reliably identify correct handlers.  Will probably need a little iteration to get the right annotations where fast natives are called.
blocking1.9.1: --- → ?
blocking1.9.2: ? → .11+
status1.9.1: --- → ?
status1.9.2: ? → wanted

Comment 5

7 years ago
Again, this is already fixed. We should DUP, not block on this one.
This testcase doesn't appear to crash 1.9.1.14pre (on mac anyway). I just repeatedly get the slow-script dialog.
(In reply to comment #5)
> Again, this is already fixed. We should DUP, not block on this one.

It's got a separate testcase, that appears to have different behavior at least on 1.9.1.x. "Depends on" is better so we don't forget to verify both testcases after the fix.
Assignee: gal → mrbkap
Depends on: 509075
Whiteboard: [sg:critical] → [sg:critical] fixed on trunk by 509075
Do we need this fix on the 1.9.1 branch too?
bug 509075 has been checked in, let's verify that was the fix in an upcoming nightly build.
blocking1.9.1: ? → .14+
status1.9.1: ? → .14-fixed
status1.9.2: wanted → .11-fixed
Alias: CVE-2010-3183
Can this be marked FIXED since it's fixed on trunk then?
Marking resolved/fixed.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 12

7 years ago
Bug 509075 is now public, so I guess this can be public now too.
Group: core-security
You need to log in before you can comment on or make changes to this bug.