The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

Core Graveyard
Installer: XPInstall Engine
RESOLVED FIXED
12 years ago
a year ago

People

(Reporter: Matthew Mastracci, Assigned: dougt)

Tracking

({fixed-aviary1.0.5, fixed1.7.9})

Trunk
x86
Linux
fixed-aviary1.0.5, fixed1.7.9
Bug Flags:
blocking1.7.9 +
blocking-aviary1.0.5 +
blocking1.8b3 +
blocking-aviary1.5 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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:
(Reporter)

Comment 1

12 years ago
Created attachment 182929 [details]
Testcase for security bug
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.
Assignee: nobody → dveditz
Severity: critical → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8b3+
Flags: blocking-aviary1.1+
Flags: blocking-aviary1.0.4?
Whiteboard: [sg:fix]
(Reporter)

Comment 3

12 years ago
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.
Flags: blocking-aviary1.0.5? → blocking-aviary1.0.5+

Comment 4

12 years ago
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.

Updated

12 years ago
Whiteboard: [sg:fix] → [sg:fix] eta: 6/28

Updated

12 years ago
Whiteboard: [sg:fix] eta: 6/28 → [sg:fix] eta: 6/28 [cb] no progress for 1.8b3?
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.
Component: Extension/Theme Manager → Installer: XPInstall Engine
Product: Firefox → Core
Version: unspecified → Trunk
Affects suite as well--worse, in fact, because there's no whitelisting.
Flags: blocking1.7.9+
(Assignee)

Updated

12 years ago
Assignee: dveditz → dougt
(Assignee)

Comment 7

12 years ago
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]

(Assignee)

Comment 8

12 years ago
dveditz tells me if you remove the IconURL from the testcase, it works as expected.
(Assignee)

Comment 9

12 years ago
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
(Assignee)

Updated

12 years ago
Attachment #188355 - Flags: review?(dveditz)
Comment on attachment 188355 [details] [diff] [review]
patch v.1

doug says he's attaching one with fixed whitespace
Attachment #188355 - Attachment is obsolete: true
Attachment #188355 - Flags: review?(dveditz)
(Assignee)

Comment 11

12 years ago
Created attachment 188362 [details] [diff] [review]
patch v.2

incorporating dan's comments.
Attachment #188362 - Flags: superreview?(jst)
Attachment #188362 - Flags: review+
Attachment #188362 - Flags: approval1.8b3?
Attachment #188362 - Flags: approval1.7.9?
Attachment #188362 - Flags: approval-aviary1.0.5?
Whiteboard: [sg:fix] eta: 6/28 [cb] no progress for 1.8b3? → [sg:fix] need sr=, a=, landing

Comment 12

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

a=chase pending jst's sr
Attachment #188362 - Flags: approval1.8b3?
Attachment #188362 - Flags: approval1.8b3+
Attachment #188362 - Flags: approval1.7.9?
Attachment #188362 - Flags: approval1.7.9+
Attachment #188362 - Flags: approval-aviary1.0.5?
Attachment #188362 - Flags: approval-aviary1.0.5+
(Reporter)

Comment 13

12 years ago
Should those sample exploit pages be password protected?  
Comment on attachment 188362 [details] [diff] [review]
patch v.2

sr=jst
Attachment #188362 - Flags: superreview?(jst) → superreview+

Updated

12 years ago
Whiteboard: [sg:fix] need sr=, a=, landing → [sg:fix] ready for landing
(Assignee)

Comment 15

12 years ago
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.
(Assignee)

Comment 16

12 years ago
Created attachment 188387 [details]
eviltrigger.html

test case
(Assignee)

Comment 17

12 years ago
Created attachment 188388 [details]
installtrigger.html

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

Comment 18

12 years ago
matthew@mastracci.com -- I removed the test cases from the web.
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
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed-aviary1.0.5, fixed1.7.9
Resolution: --- → FIXED
Whiteboard: [sg:fix] ready for landing → [sg:fix]
Blocks: 256195
Adding distributors
Security advisories published
Group: security

Updated

11 years ago
Flags: testcase+

Updated

10 years ago
Flags: in-testsuite+ → in-testsuite?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.