Last Comment Bug 598669 - (CVE-2010-3183) LookupGetterOrSetter Remote Code Execution Vulnerability (ZDI-CAN-929)
: LookupGetterOrSetter Remote Code Execution Vulnerability (ZDI-CAN-929)
[sg:critical] fixed on trunk by 509075
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 1.9.2 Branch
: All All
-- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Jason Orendorff [:jorendorff]
Depends on: 509075
  Show dependency treegraph
Reported: 2010-09-22 09:39 PDT by Brandon Sterne (:bsterne)
Modified: 2010-11-06 14:58 PDT (History)
12 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (crashes 1.9.2) (1.62 KB, text/html)
2010-09-22 09:39 PDT, Brandon Sterne (:bsterne)
no flags Details

Description User image Brandon Sterne (:bsterne) 2010-09-22 09:39:39 PDT
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:

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),

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,
(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
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
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 User image Andreas Gal :gal 2010-09-22 11:16:44 PDT
Fixed by bug 509075 I believe. Deferring to mrbkap and jorendorff for final confirmation.
Comment 2 User image Brendan Eich [:brendan] 2010-09-22 11:27:24 PDT
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.

Comment 3 User image Andreas Gal :gal 2010-09-22 20:13:15 PDT
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 User image Brian Hackett (:bhackett) 2010-09-22 20:32:45 PDT
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 User image Andreas Gal :gal 2010-09-24 11:21:03 PDT
Again, this is already fixed. We should DUP, not block on this one.
Comment 6 User image Daniel Veditz [:dveditz] 2010-09-24 12:22:28 PDT
This testcase doesn't appear to crash (on mac anyway). I just repeatedly get the slow-script dialog.
Comment 7 User image Daniel Veditz [:dveditz] 2010-09-24 12:31:07 PDT
(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 User image Daniel Veditz [:dveditz] 2010-09-27 16:17:16 PDT
Do we need this fix on the 1.9.1 branch too?
Comment 9 User image Daniel Veditz [:dveditz] 2010-09-28 23:39:04 PDT
bug 509075 has been checked in, let's verify that was the fix in an upcoming nightly build.
Comment 10 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-10-12 14:09:14 PDT
Can this be marked FIXED since it's fixed on trunk then?
Comment 11 User image Damon Sicore (:damons) 2010-10-19 13:59:56 PDT
Marking resolved/fixed.
Comment 12 User image Jesse Ruderman 2010-11-06 14:58:27 PDT
Bug 509075 is now public, so I guess this can be public now too.

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