598669 - (CVE-2010-3183) LookupGetterOrSetter Remote Code Execution Vulnerability (ZDI-CAN-929)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brandon Sterne (:bsterne)]

Attachment: 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

Comment 1

[Andreas Gal :gal]

Fixed by bug 509075 I believe. Deferring to mrbkap and jorendorff for final confirmation.

Comment 2

[Brendan Eich [:brendan]]

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

[Andreas Gal :gal]

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.

Comment 4

[Brian Hackett [Laid off!]]

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.

Comment 5

[Andreas Gal :gal]

Again, this is already fixed. We should DUP, not block on this one.

Comment 6

[Daniel Veditz [:dveditz]]

This testcase doesn't appear to crash 1.9.1.14pre (on mac anyway). I just repeatedly get the slow-script dialog.

Comment 7

[Daniel Veditz [:dveditz]]

(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.

Comment 8

[Daniel Veditz [:dveditz]]

Do we need this fix on the 1.9.1 branch too?

Comment 9

[Daniel Veditz [:dveditz]]

bug 509075 has been checked in, let's verify that was the fix in an upcoming nightly build.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Can this be marked FIXED since it's fixed on trunk then?

Comment 11

[Damon Sicore (:damons)]

Marking resolved/fixed.

Comment 12

[Jesse Ruderman]

Bug 509075 is now public, so I guess this can be public now too.