Closed
Bug 295854
Opened 19 years ago
Closed 19 years ago
(new InstallVersion()).compareTo(/x/) crashes [@ConvertJSValToObj]
Categories
(Core Graveyard :: Installer: XPInstall Engine, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: sync2d, Assigned: timeless)
References
()
Details
(4 keywords, Whiteboard: [sg:fix])
Crash Data
Attachments
(1 file, 5 obsolete files)
1.10 KB,
text/html
|
Details |
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.
Comment 1•19 years ago
|
||
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
Severity: normal → critical
Summary: (new InstallVersion()).compareTo(/x/) crashes → (new InstallVersion()).compareTo(/x/) crashes [@ConvertJSValToObj]
Whiteboard: testcase crash
Comment 2•19 years ago
|
||
TB6213255E for me
Comment 3•19 years ago
|
||
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.
Assignee: xpi-engine → timeless
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.
Attachment #184810 -
Flags: superreview?(dveditz)
Attachment #184810 -
Flags: review?(dveditz)
[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).
Updated•19 years ago
|
Flags: blocking1.8b3+
Flags: blocking1.7.9+
Flags: blocking-aviary1.0.5+
Whiteboard: [sg:fix]
Updated•19 years ago
|
Whiteboard: [sg:fix] → [sg:fix] have patch- - needs review
Comment 10•19 years ago
|
||
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.
Attachment #184810 -
Flags: superreview?(dveditz)
Attachment #184810 -
Flags: review?(dveditz)
Attachment #184810 -
Flags: review-
Comment 11•19 years ago
|
||
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 12•19 years ago
|
||
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.
Comment 13•19 years ago
|
||
Timeless: Please take a look and see if you can get a new patch reviewed soon. Thanks.
Assignee | ||
Comment 14•19 years ago
|
||
Attachment #184810 -
Attachment is obsolete: true
Attachment #186432 -
Flags: superreview?(dveditz)
Attachment #186432 -
Flags: review?(dveditz)
Comment 15•19 years ago
|
||
Comment on attachment 186432 [details] [diff] [review] use JS_InstanceOf r/sr=dveditz a=dveditz for branch landing
Attachment #186432 -
Flags: superreview?(dveditz)
Attachment #186432 -
Flags: superreview+
Attachment #186432 -
Flags: review?(dveditz)
Attachment #186432 -
Flags: review+
Attachment #186432 -
Flags: approval1.7.9+
Attachment #186432 -
Flags: approval-aviary1.0.5+
Comment 16•19 years ago
|
||
Comment on attachment 186432 [details] [diff] [review] use JS_InstanceOf r/sr=dveditz a=dveditz for branch landing
Attachment #186432 -
Flags: approval1.7.9+
Attachment #186432 -
Flags: approval-aviary1.0.5+
Updated•19 years ago
|
Attachment #186432 -
Flags: approval1.7.9+
Attachment #186432 -
Flags: approval-aviary1.0.5+
Comment 17•19 years ago
|
||
Sorry for the double-submit, bugzilla returned a 500 error so I thought the first didn't go through.
Reporter | ||
Comment 18•19 years ago
|
||
> (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
Assignee | ||
Comment 19•19 years ago
|
||
Attachment #186450 -
Flags: superreview?(dveditz)
Attachment #186450 -
Flags: review?(dveditz)
Assignee | ||
Comment 20•19 years ago
|
||
if things ever need the function, the prototype really should be moved to a header file.
Attachment #186452 -
Flags: superreview?(dveditz)
Attachment #186452 -
Flags: review?(dveditz)
Updated•19 years ago
|
Attachment #186432 -
Flags: approval1.8b3+
Comment 21•19 years ago
|
||
Comment on attachment 186452 [details] [diff] [review] remove extra definitions r/sr/a=dveditz
Attachment #186452 -
Flags: superreview?(dveditz)
Attachment #186452 -
Flags: superreview+
Attachment #186452 -
Flags: review?(dveditz)
Attachment #186452 -
Flags: review+
Attachment #186452 -
Flags: approval1.8b3+
Attachment #186452 -
Flags: approval1.7.9+
Attachment #186452 -
Flags: approval-aviary1.0.5+
Comment 22•19 years ago
|
||
Comment on attachment 186450 [details] [diff] [review] nsisupports marker r/sr/a=dveditz
Attachment #186450 -
Flags: superreview?(dveditz)
Attachment #186450 -
Flags: superreview+
Attachment #186450 -
Flags: review?(dveditz)
Attachment #186450 -
Flags: review+
Attachment #186450 -
Flags: approval1.8b3+
Attachment #186450 -
Flags: approval1.7.9+
Attachment #186450 -
Flags: approval-aviary1.0.5+
Updated•19 years ago
|
Whiteboard: [sg:fix] have patch- - needs review → [sg:fix] need landing
Assignee | ||
Comment 23•19 years ago
|
||
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
Attachment #186432 -
Attachment is obsolete: true
Assignee | ||
Comment 24•19 years ago
|
||
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
Attachment #186450 -
Attachment is obsolete: true
Assignee | ||
Comment 25•19 years ago
|
||
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
Attachment #186452 -
Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed-aviary1.0.5,
fixed1.7.9
Resolution: --- → FIXED
Whiteboard: [sg:fix] need landing
Reporter | ||
Comment 26•19 years ago
|
||
javascript: (new InstallVersion()).compareTo(null); still crashes.
Assignee | ||
Comment 27•19 years ago
|
||
Attachment #186542 -
Flags: superreview?(dveditz)
Attachment #186542 -
Flags: review?(dveditz)
Reporter | ||
Comment 28•19 years ago
|
||
> (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 29•19 years ago
|
||
Comment on attachment 186542 [details] [diff] [review] handle null r/sr=dveditz
Attachment #186542 -
Flags: superreview?(dveditz)
Attachment #186542 -
Flags: superreview+
Attachment #186542 -
Flags: review?(dveditz)
Attachment #186542 -
Flags: review+
Comment 30•19 years ago
|
||
shutdown: mail to you is bouncing, please contact chofmann@mozilla.org
Comment 31•19 years ago
|
||
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.
Reporter | ||
Comment 32•19 years ago
|
||
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+
Attachment #186542 -
Flags: approval1.8b3?
Comment 33•19 years ago
|
||
(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 34•19 years ago
|
||
Comment on attachment 186542 [details] [diff] [review] handle null a=jay for landing on trunk and branches (driver mtg)
Attachment #186542 -
Flags: approval1.8b3?
Attachment #186542 -
Flags: approval1.8b3+
Attachment #186542 -
Flags: approval1.7.9+
Attachment #186542 -
Flags: approval-aviary1.0.5+
Assignee | ||
Comment 35•19 years ago
|
||
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
Attachment #186542 -
Attachment is obsolete: true
Comment 36•19 years ago
|
||
Adding distributors
Comment 38•19 years ago
|
||
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?
Updated•19 years ago
|
Group: security
Updated•19 years ago
|
Group: security
Updated•19 years ago
|
Flags: testcase+
Updated•17 years ago
|
Flags: in-testsuite+ → in-testsuite?
Comment 39•15 years ago
|
||
in-testsuite-: InstallVersion is no more.
Flags: in-testsuite? → in-testsuite-
Updated•13 years ago
|
Crash Signature: [@ConvertJSValToObj]
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•