Last Comment Bug 293331 - InstallTrigger may run callback on a different page than the page that triggered the install.
: InstallTrigger may run callback on a different page than the page that trigge...
Status: RESOLVED FIXED
[sg:fix]
: fixed-aviary1.0.5, fixed1.7.9
Product: Core Graveyard
Classification: Graveyard
Component: Installer: XPInstall Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Doug Turner (:dougt)
:
Mentors:
Depends on:
Blocks: sbb?
  Show dependency treegraph
 
Reported: 2005-05-08 01:09 PDT by Matthew Mastracci
Modified: 2015-12-11 07:21 PST (History)
10 users (show)
dveditz: blocking1.7.9+
dveditz: blocking‑aviary1.0.5+
dveditz: blocking1.8b3+
dveditz: blocking‑aviary1.5+
bob: in‑testsuite?
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Testcase for security bug (385 bytes, text/html)
2005-05-08 01:10 PDT, Matthew Mastracci
no flags Details
patch v.1 (7.48 KB, patch)
2005-07-05 14:33 PDT, Doug Turner (:dougt)
no flags Details | Diff | Splinter Review
patch v.2 (8.67 KB, patch)
2005-07-05 14:51 PDT, Doug Turner (:dougt)
dveditz: review+
jst: superreview+
chase: approval‑aviary1.0.5+
chase: approval1.7.9+
chase: approval1.8b3+
Details | Diff | Splinter Review
eviltrigger.html (344 bytes, text/html)
2005-07-05 18:33 PDT, Doug Turner (:dougt)
no flags Details
installtrigger.html (185 bytes, text/html)
2005-07-05 18:34 PDT, Doug Turner (:dougt)
no flags Details

Description Matthew Mastracci 2005-05-08 01:09:57 PDT
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 security@mozilla.org:

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 2005-05-08 01:10:33 PDT
Created attachment 182929 [details]
Testcase for security bug
Comment 2 Daniel Veditz [:dveditz] 2005-05-08 14:14:42 PDT
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 2005-05-09 07:43:56 PDT
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] 2005-06-23 10:28:06 PDT
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] 2005-06-30 12:49:19 PDT
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] 2005-06-30 13:48:53 PDT
Affects suite as well--worse, in fact, because there's no whitelisting.
Comment 7 Doug Turner (:dougt) 2005-07-05 13:57:39 PDT
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) 2005-07-05 14:08:08 PDT
dveditz tells me if you remove the IconURL from the testcase, it works as expected.
Comment 9 Doug Turner (:dougt) 2005-07-05 14:33:37 PDT
Created attachment 188355 [details] [diff] [review]
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] 2005-07-05 14:50:50 PDT
Comment on attachment 188355 [details] [diff] [review]
patch v.1

doug says he's attaching one with fixed whitespace
Comment 11 Doug Turner (:dougt) 2005-07-05 14:51:24 PDT
Created attachment 188362 [details] [diff] [review]
patch v.2

incorporating dan's comments.
Comment 12 Chase Phillips 2005-07-05 15:00:28 PDT
Comment on attachment 188362 [details] [diff] [review]
patch v.2

a=chase pending jst's sr
Comment 13 Matthew Mastracci 2005-07-05 15:04:59 PDT
Should those sample exploit pages be password protected?  
Comment 14 Johnny Stenback (:jst, jst@mozilla.com) 2005-07-05 15:24:49 PDT
Comment on attachment 188362 [details] [diff] [review]
patch v.2

sr=jst
Comment 15 Doug Turner (:dougt) 2005-07-05 18:32:42 PDT
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) 2005-07-05 18:33:40 PDT
Created attachment 188387 [details]
eviltrigger.html

test case
Comment 17 Doug Turner (:dougt) 2005-07-05 18:34:32 PDT
Created attachment 188388 [details]
installtrigger.html

installtrigger.install() example which should work -- it does nothing evil.
Comment 18 Doug Turner (:dougt) 2005-07-05 18:36:10 PDT
matthew@mastracci.com -- I removed the test cases from the web.
Comment 19 Daniel Veditz [:dveditz] 2005-07-05 22:56:52 PDT
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] 2005-07-12 11:35:12 PDT
Adding distributors
Comment 21 Daniel Veditz [:dveditz] 2005-07-12 18:05:12 PDT
Security advisories published

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