293331 - InstallTrigger may run callback on a different page than the page that triggered the install.

Status: RESOLVED FIXED

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

Description

[Matthew Mastracci]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050416 Fedora/1.0.3-1.3.1 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050416 Fedora/1.0.3-1.3.1 Firefox/1.0.3

My report to [e-mail]:

An attacker may force a script installation to happen, then change the page
behind the scenes so that the callback occurs in the domain of any random page.
 Note that this happens whether the user clicks "ok" or "cancel" on the XPI
install dialog.  This can be used to steal cookies, passwords, submit
information to another site, etc.  The sample exploit provides the users
bugzilla cookie in a dialog, but could easily submit it to another site. 

See attached testcase.  Note that testcase should be run on a different server,
since it steals cookies from bugzilla.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Matthew Mastracci]

Attachment: Testcase for security bug

Comment 2

[Daniel Veditz [:dveditz]]

Exploitation would require injecting this code into a site whitelisted for
install (but that's currently possible, see bug 292302). Then again, if you can
inject code into arbitrary sites you could just inject the attack code rather
than go through the InstallTrigger redirection.

Comment 3

[Matthew Mastracci]

The worse case is being able to inject code in about:config or any other XUL
page, if you can get around the problem of loading that sort of page from
untrusted content.  The InstallTrigger script would be run within the chrome,
with full chrome privileges.

Since you can inject code into any page (even those not normally vulnerable to
injection) I think this becomes much more critical.

Comment 4

[Jay Patel [:jay]]

cc'ing a few folks to see if we can get some traction here, we need a fix soon
if this is going to make 1.0.5.

Comment 5

[Daniel Veditz [:dveditz]]

I was hoping a change for an underlying frame transition bug would help me out,
but that has been pushed out to the next release due to risk. Will have to do a
one-off hack for this one.

The testcase as provided fails because 1.0.4 removed the ability to have
javascript: IconURLs. The problem doesn't rely on that, however -- the bug is
still there if you modify the testcase.

Note this is more of a problem for 1.7.x -- there's no xpinstall whitelist to
prevent anyone from doing this whenever they want. In Firefox this is not that
big a practical worry unless we find another code-injection bug, and if we have
one of those then you don't need this exploit to do the code-injection.

Comment 6

[Daniel Veditz [:dveditz]]

Affects suite as well--worse, in fact, because there's no whitelisting.

Comment 7

[Doug Turner (:dougt)]

The testcase is bad:

Error: uncaught exception: [Exception... "Could not convert JavaScript argument
arg 0 [nsIDOMLocation.href]"  nsresult: "0x80570009
(NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame ::
javascript:location.href=new function() { this.toString = function() { var x = {
'test': { URL: 'foo.xpi', IconURL: 'javascript:alert(\'test\');' }
};InstallTrigger.install(x,function() { if
(location.host=='bugzilla.mozilla.org') alert('Your Bugzilla cookies are: ' +
document.cookie); } ); return 'http://bugzilla.mozilla.org'; }; } :: <TOP_LEVEL>
:: line 1"  data: no]

Comment 8

[Doug Turner (:dougt)]

dveditz tells me if you remove the IconURL from the testcase, it works as expected.

Comment 9

[Doug Turner (:dougt)]

Attachment: patch v.1

The approach taken is to save the nsIPrincipal when InstallTrigger.install is
called.  Then before calling out to the saved callback, we check to see if the
current principal matches what we saved.  This ensures that InstallTrigger and
the callback function exist within the same context, if i understand correctly.


This patch provide no user interface that the callback was not executed.


I have tested this against a modified version of the testcase above which I put
here for testing.

http://www.meer.net/~dougt/eviltrigger.html

I also put a legal "do no evil" test case which shouldn't be broken here.  You
will see a install fail dialog -- but the point is that the callback is called
and you will see an alert telling you that.

http://www.meer.net/~dougt/installtrigger.html

In both cases, you need to whitelist www.meer.net for _testing_.

Doug

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 188355 [details] [diff] [review]
patch v.1

doug says he's attaching one with fixed whitespace

Comment 11

[Doug Turner (:dougt)]

Attachment: patch v.2

incorporating dan's comments.

Comment 12

[Chase Phillips]

Comment on attachment 188362 [details] [diff] [review]
patch v.2

a=chase pending jst's sr

Comment 13

[Matthew Mastracci]

Should those sample exploit pages be password protected?  

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 188362 [details] [diff] [review]
patch v.2

sr=jst

Comment 15

[Doug Turner (:dougt)]

TRUNK:

Checking in nsJSInstallTriggerGlobal.cpp;
/cvsroot/mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp,v  <-- 
nsJSInstallTriggerGlobal.cpp
new revision: 1.46; previous revision: 1.45
done
Checking in nsXPITriggerInfo.cpp;
/cvsroot/mozilla/xpinstall/src/nsXPITriggerInfo.cpp,v  <--  nsXPITriggerInfo.cpp
new revision: 1.29; previous revision: 1.28
done
Checking in nsXPITriggerInfo.h;
/cvsroot/mozilla/xpinstall/src/nsXPITriggerInfo.h,v  <--  nsXPITriggerInfo.h
new revision: 1.21; previous revision: 1.20
done

AVIARY_1_0_1_20050124_BRANCH:

Checking in nsJSInstallTriggerGlobal.cpp;
/cvsroot/mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp,v  <-- 
nsJSInstallTriggerGlobal.cpp
new revision: 1.35.6.4.2.5; previous revision: 1.35.6.4.2.4
done
Checking in nsXPITriggerInfo.cpp;
/cvsroot/mozilla/xpinstall/src/nsXPITriggerInfo.cpp,v  <--  nsXPITriggerInfo.cpp
new revision: 1.24.16.2; previous revision: 1.24.16.1
done
Checking in nsXPITriggerInfo.h;
/cvsroot/mozilla/xpinstall/src/nsXPITriggerInfo.h,v  <--  nsXPITriggerInfo.h
new revision: 1.18.16.2; previous revision: 1.18.16.1
done

MOZILLA_1_7_BRANCH:

Checking in nsJSInstallTriggerGlobal.cpp;
/cvsroot/mozilla/xpinstall/src/nsJSInstallTriggerGlobal.cpp,v  <-- 
nsJSInstallTriggerGlobal.cpp
new revision: 1.35.2.7; previous revision: 1.35.2.6
done
Checking in nsXPITriggerInfo.cpp;
/cvsroot/mozilla/xpinstall/src/nsXPITriggerInfo.cpp,v  <--  nsXPITriggerInfo.cpp
new revision: 1.24.2.2; previous revision: 1.24.2.1
done
Checking in nsXPITriggerInfo.h;
/cvsroot/mozilla/xpinstall/src/nsXPITriggerInfo.h,v  <--  nsXPITriggerInfo.h
new revision: 1.18.2.2; previous revision: 1.18.2.1
done

Marking fixed.

Comment 16

[Doug Turner (:dougt)]

Attachment: eviltrigger.html

test case

Comment 17

[Doug Turner (:dougt)]

Attachment: installtrigger.html

installtrigger.install() example which should work -- it does nothing evil.

Comment 18

[Doug Turner (:dougt)]

[e-mail] -- I removed the test cases from the web.

Comment 19

[Daniel Veditz [:dveditz]]

Note to QA: the eviltrigger.html testcase (or original testcase for Firefox
1.0.3 or earlier) need to be hosted on a non-bugzilla site -- if the original
page starts on bugzilla then there's no problem if the callback can get your
bugzilla cookies.

Adding fixed keywords to match checkin comment

Comment 20

[Daniel Veditz [:dveditz]]

Adding distributors

Comment 21

[Daniel Veditz [:dveditz]]

Security advisories published