298054 - eval(string) crashes in XPInstall [@ nsInstall::~nsInstall()]

Status: RESOLVED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect)

Description

[Karsten Düsterloh]

Calling eval with a string argument in an install.js XPInstall script crashes
Mozilla (it doesn't for numeric arguments to eval).

Steps to reproduce:
- take any addon and modify its install.js to begin with
    alert("before");
    eval("some string");
    alert("after");
- try to install the modified addon:
  * before alert pops up
  * Mozilla crashes

Mozilla trunk build regression range is between 2005061507 (okay) and 2005061605
(broken), judging by the respective checkins I suspect bug 295854 (again, not
readable by me) as (part of) the cause.

Related Talkback data: TB6737141Z, TB6737120K.

Tobias, can you test your 2005061518 to split the regression range and
confirm/deny my suspect?

Comment 1

[Karsten Düsterloh]

Win2k/MSVC crash stack of my today's debug build:

nsInstall::~nsInstall() line 283 + 12 bytes
nsInstall::`scalar deleting destructor'(unsigned int 0x03aa6050) + 15 bytes
nsQueryInterface::operator()(const nsID & {...}, void * * 0x0483f200) line 47 +
23 bytes
nsCOMPtr<nsIXPConnectWrappedNative>::assign_from_qi(nsQueryInterface {...},
const nsID & {...}) line 1232 + 17 bytes
nsCOMPtr<nsIXPConnectWrappedNative>::nsCOMPtr<nsIXPConnectWrappedNative>(nsQueryInterface
{...}) line 646
nsScriptSecurityManager::doGetObjectPrincipal(JSContext * 0x03a90e88, JSObject *
0x03977568) line 2020
nsScriptSecurityManager::GetObjectPrincipal(nsScriptSecurityManager * const
0x00f5cf18, JSContext * 0x03a90e88, JSObject * 0x03aac140, nsIPrincipal * *
0x0483f280) line 1987 + 13 bytes
ObjectPrincipalFinder(JSContext * 0x03a90e88, JSObject * 0x03aac140) line 2131 +
49 bytes
JS_EvalFramePrincipals(JSContext * 0x03a90e88, JSStackFrame * 0x0483f3ac,
JSStackFrame * 0x0483fda4) line 707 + 26 bytes
obj_eval(JSContext * 0x03a90e88, JSObject * 0x03977568, unsigned int 0x00000001,
long * 0x03ae4a40, long * 0x0483f3cc) line 1092 + 17 bytes
js_Invoke(JSContext * 0x03a90e88, unsigned int 0x00000001, unsigned int
0x00000000) line 1178 + 23 bytes
js_Interpret(JSContext * 0x03a90e88, unsigned char * 0x03ae3b4b, long *
0x0483fdc4) line 3468 + 15 bytes
js_Execute(JSContext * 0x03a90e88, JSObject * 0x03977568, JSScript * 0x03ae3aa0,
JSStackFrame * 0x00000000, unsigned int 0x00000000, long * 0x0483ff18) line 1408
+ 19 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x03a90e88, JSObject * 0x03977568,
JSPrincipals * 0x00000000, const unsigned short * 0x03acb610, unsigned int
0x000035be, const char * 0x00000000, unsigned int 0x00000000, long * 0x0483ff18)
line 3879 + 25 bytes
JS_EvaluateUCScript(JSContext * 0x03a90e88, JSObject * 0x03977568, const
unsigned short * 0x03acb610, unsigned int 0x000035be, const char * 0x00000000,
unsigned int 0x00000000, long * 0x0483ff18) line 3857 + 35 bytes
JS_EvaluateScript(JSContext * 0x03a90e88, JSObject * 0x03977568, const char *
0x03a93ae8, unsigned int 0x000035be, const char * 0x00000000, unsigned int
0x00000000, long * 0x0483ff18) line 3824 + 33 bytes
RunInstallOnThread(void * 0x0353f798) line 559 + 30 bytes
_PR_NativeRunThread(void * 0x035405d8) line 436 + 13 bytes
pr_root(void * 0x035405d8) line 112 + 13 bytes
_threadstartex(void * 0x035407a8) line 212 + 13 bytes
KERNEL32! 77e8758a()

Comment 2

[Karsten Düsterloh]

oops, wrong CC

Comment 3

[Tobias Fischer]

(In reply to comment #0)

> Tobias, can you test your 2005061518 to split the regression range and
> confirm/deny my suspect?

Just testing XP-Install with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.8b2) Gecko/20050615 {Build ID: 2005061518} installing the Mnenhy.xpi and it
works almost fine. 
Sure 2005061605 crashes when trying to install, so the regression range is
between 2005061518 and 2005061605.

Comment 4

[Karsten Düsterloh]

Attachment: minimal crash XPI

This XPI does only contain an install.js with the three lines mentioned in
comment #0. When trying to install it into a current Mozilla trunk build, it
will alert "before" and then crash...

Comment 5

[Karsten Düsterloh]

Attachment: minimal crash XPI 'evalcrash.xpi' (without syntax error)

I backed out both parts of bug 295854 checked in in the regression range
(<http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-06-15+21%3A40%3A00&maxdate=2005-06-16+04%3A15%3A00&cvsroot=%2Fcvsroot>)

in my tree and now evalcrash.xpi completes without error.

Comment 6

[Karsten Düsterloh]

Attachment: remove JSCLASS_PRIVATE_IS_NSISUPPORTS from InstallClass

I dug into that this morning and found that the first patch mentioned in the
regression range made InstallClass advertize JSCLASS_PRIVATE_IS_NSISUPPORTS -
but the nsInstall class isn't derived from nsISupports, thus when trying to QI
the private everything blows up - the magic foo of the JS engine assumes that
vtable[0] is the QueryInterface function, but it's the nsInstall dtor...

I waylayed timeless on IRC and he told me to remove the misleading ad above -
and that does help indeed! However, I can't tell if that has any impact upon
the security bug's state...

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 187052 [details] [diff] [review]
remove JSCLASS_PRIVATE_IS_NSISUPPORTS from InstallClass

shutdown provided the same patch in the other bug. r=timeless (via irc),
sr=dveditz
a=dveditz for everywhere.

Comment 8

[Daniel Veditz [:dveditz]]

Fix checked in to trunk and branches. Thanks to shutdown and Mnyromyr for
fingering the problem.