Last Comment Bug 672485 - Holding enter allows arbitrary extension installation.
: Holding enter allows arbitrary extension installation.
Status: VERIFIED FIXED
[sg:critical][qa!]
: verified-aurora, verified-beta
Product: Firefox
Classification: Client Software
Component: Security (show other bugs)
: 5 Branch
: x86 Windows 7
: -- critical (vote)
: Firefox 9
Assigned To: Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not)
:
Mentors:
Depends on: 682410 655916
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-19 06:34 PDT by Mariusz Mlynski
Modified: 2014-06-26 09:35 PDT (History)
14 users (show)
rforbes: sec‑bounty+
bmcbride: in‑testsuite+
bmcbride: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
-
wontfix
+
fixed
+
fixed
+
fixed
unaffected


Attachments
The certificate to import. (883 bytes, text/plain)
2011-07-19 06:35 PDT, Mariusz Mlynski
no flags Details
Test focusing pluginspage install link in Nightly build. (1.07 KB, text/html)
2011-07-21 21:18 PDT, Mariusz Mlynski
no flags Details
Patch v1 (4.40 KB, patch)
2011-08-04 19:40 PDT, Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not)
dtownsend: review+
jst: review+
Details | Diff | Splinter Review
Patch v1.1 (7.90 KB, patch)
2011-08-22 20:16 PDT, Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not)
dtownsend: review+
asa: approval‑mozilla‑aurora+
christian: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Screenshot: Test (163.10 KB, image/png)
2011-09-30 17:10 PDT, Anthony Hughes (:ashughes) [GFX][QA][Mentor]
no flags Details

Description Mariusz Mlynski 2011-07-19 06:34:26 PDT
It is possible to install an arbitrary XPI without user's consent if the user holds Enter on a malicious website containing a specially crafted JavaScript code.

There are 2 layers of protection against an unauthorized installation of extensions:

1) The principal of the opener is checked against whitelisted domains that are allowed to download the plugin without asking. If the domain is not trusted, the user is asked to allow to download the plugin.
2) When the plugin is downloaded, the user is asked to confirm the installation.

The first protection can be circumvented by creating a hidden "Embed" element containing an arbitrary XPI as its "pluginspage" parameter. The attacker can focus this element while the user holds Enter, causing a number of "Plugin Finder Service" windows to appear. The first window focuses the "Cancel" button and will just close, but all the subsequent ones will set focus on the "Manual Install" button directing to the malicious XPI. As soon as the user releases the key, the browser will start launching multiple windows with the provided URL. The windows will have a ChromeWindow object as their opener, so the user will not be asked to allow to download a plugin.

The second protection can be bypassed due to a logic error in amWebInstallListener.js. When no window-watcher is registered in Services, this will throw:

Services.ww.openWindow(this.window, "chrome://mozapps/content/xpinstall/xpinstallConfirm.xul",null, "chrome,modal,centerscreen", args);

with the following message in the console:

Warning: WARN addons.manager: InstallListener threw exception when calling onDownloadEnded: TypeError: Services.ww is undefined
Source File: resource://gre/components/amWebInstallListener.js
Line: 170

When the confirmation dialog fails to open, the installation will commence as though the user clicked "Install".

Getting the browser into a state in which Services.ww is permanently undefined can be achieved with no user interaction by using a modified version of https://bugzilla.mozilla.org/show_bug.cgi?id=616659 on a signed website. Exploiting it locally will likely bring about corrupt windows and a lot of errors in the console, but Services.ww will be defined. On a remote signed website, with the try-catch-inside-eval recursion performed during page load, after an alert dialog which prevents "too much recursion" error from occuring, it fails silently and Services.ww remains undefined for the whole browsing session, ie. Firefox will not ask to confirm a plugin installation until the user restarts the browser. The only mitigating factor is that if Services.ww was defined earlier in the browser session, the exploit may fail and corrupt windows will appear (hence point #2 in the steps described below, after opening windows to import the certificate). This was noticed by Zach Hoffman in https://bugzilla.mozilla.org/show_bug.cgi?id=616659#c37 but the implications there were reverse.

There may be other ways to wind up with Services.ww undefined, but those should be safe as long as the plugin installation is stopped when xpinstallConfirm.xul fails to open.

Reproducible: always (as long as Services.ww wasn't defined earlier in the browsing session)

Steps to reproduce:
1. Import x509.cert for software makers identification.
2. Close the browser and reopen after the process shuts down.
3. Open jar:http://x.x.x.x/mozilla/signed.jar!exploit.htm (see comment 1)
4. Press and hold enter.

Actual results:
The plugin is automatically downloaded and installed.

Expected results:
The plugin should neither be downloaded nor installed without user's consent.

Verified to work with Firefox 5.0.1 on Windows 7 SP1 64-bit and Windows XP SP3 32-bit.
Comment 2 Mariusz Mlynski 2011-07-19 06:35:55 PDT
Created attachment 546770 [details]
The certificate to import.
Comment 3 Mariusz Mlynski 2011-07-19 06:37:34 PDT
Created attachment 546773 [details]
Proof of concept - backup.
Comment 4 Mariusz Mlynski 2011-07-19 06:39:11 PDT
Ah, one more thing: if the exploit fails, ie. Services.ww happens to be already defined in the browsing session causing display of corrupt windows after loading the page, and you deny privileges to stop windows from popping-up, then make sure to remove these lines from prefs.js before retrying:

user_pref("capability.principal.certificate.pX.denied", "UniversalXPConnect");
user_pref("capability.principal.certificate.pX.id", "BF:94:EA:8A:E8:55:BC:EE:59:0C:4B:0B:6D:3B:09:80:9D:9D:F7:0E");
user_pref("capability.principal.certificate.pX.subjectName", "CN=a,O=a,OU=a,ST=a,C=aa,UID=a,E=a");

Also, I found that "pluginspage" windows open a bit slow with with ChromeBug 1.7.2 installed, so be advised to turn it off if you have it enabled.
Comment 6 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-07-19 06:55:18 PDT
This is a pretty confused bug report. If you give any website UniversalXPConnect, you've lost (there are much easier ways to exploit a user once you've got UniversalXPConnect). We're removing support for enablePrivilege, so I don't think that part of the bug is a concern.

It's also not clear to me: are you saying that bug 616659 is not fixed?

Nevertheless, there are a few bits of this report that do concern me:

* embed pluginspage= can bypass the XPI whitelist (an annoyance hole, but not strictly a security bug)
* the pluginspage frame opens with a ChromeWindow opener (a security bug, but one I think we will have fixed in FF6 or 7)
* If the confirmation dialog fails, we should just stop, rather than continuing
Comment 7 Mariusz Mlynski 2011-07-19 07:14:35 PDT
OK, let me rephrase:

If you run the following code:

alert('Welcome aboard!');
function xpl(){
	eval("try{xpl()}catch(b){netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect')}");
}
try{xpl()}catch(e){};

then, provided that Services.ww wasn't defined earlier in the browsing session, no windows asking for an elevated privilege will appear. The browser will fail to display any windows, and the user will not be asked for an elevated privilege! Instead, the browser will be in a state in which Services.ww is not defined, which allows to install a plug-in without asking due to a bug in resource://gre/components/amWebInstallListener.js. My report has nothing to do with granting UniversalXPConnect privileges, nor it is required that the user grants any elevated privileges, please reread it.

This bug: https://bugzilla.mozilla.org/show_bug.cgi?id=616659 is fixed in that corrupt windows that may appear (if Services.ww was defined earlier in the browsing session) will not evaluate to true when closed. While fiddling with that code, I found that if the windows fail to display, Services.ww is undefined, which allows to install plugins without any confirmation.

I can provide the video of running the exploit if needed.
Comment 8 Dave Townsend [:mossop] 2011-07-19 09:37:41 PDT
(In reply to comment #7)
> This bug: https://bugzilla.mozilla.org/show_bug.cgi?id=616659 is fixed in
> that corrupt windows that may appear (if Services.ww was defined earlier in
> the browsing session) will not evaluate to true when closed. While fiddling
> with that code, I found that if the windows fail to display, Services.ww is
> undefined, which allows to install plugins without any confirmation.

It's not clear to me how you're making Services.ww undefined. Is there some example code that does that?
Comment 9 Mariusz Mlynski 2011-07-19 10:14:08 PDT
As stated in the first post:

"On a remote signed website, with the try-catch-inside-eval recursion performed during page load, after an alert dialog which prevents "too much recursion" error from occuring, it fails silently and Services.ww remains undefined for the whole browsing session"

Where "it" is the following code:

function xpl(){
	eval("try{xpl()}catch(b){netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect')}");
}
try{xpl()}catch(e){};

Thus, the effective script is:

<script>
alert('anything');
function xpl(){
	eval("try{xpl()}catch(b){netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect')}");
}
try{xpl()}catch(e){};
</script>

If it's run on a remote signed website before any other windows (not tabs) were launched in the browsing session, then Services.ww remains undefined for the rest of this browsing session, which in turn allows to install plugins without confirming.
Comment 10 Mariusz Mlynski 2011-07-19 12:10:41 PDT
Btw, jar:http:...signed.jar!/exploit.htm linked in comment 5 does exactly that on load, after closing the pop-up. On all computers (Win XP/7) I've tried, Services.ww was permanently undefined after following the first 3 steps from comment 0 (+closing the "welcome" alert), and Firefox did not ask to confirm installation (xpinstallConfirm.xul) after I held and released Enter or navigated from the exploit page to AMO and clicked any of the "add to Firefox" buttons. With a malicious third party with an access to a proper certificate from a trusted CA, just steps 3 and 4 are necessary to have an arbitrary extension installed in the browser.
Comment 11 Dave Townsend [:mossop] 2011-07-19 15:04:12 PDT
So far on OSX the only result of running this code is a stalled browser that won't respond to input, no extension install.
Comment 12 Dave Townsend [:mossop] 2011-07-19 15:28:19 PDT
On Windows 7 the testcase did something more, after holding enter for a while the install missing plugins bar appeared and it attempted to stop me leaving the page, but no add-on got downloaded that I can see, Services.ww was showing as defined after that.
Comment 13 Mariusz Mlynski 2011-07-20 02:22:53 PDT
Dave, what version did you check? The proof of concept is suited for the current public release (5.0.1).

I've made 2 short videos to show how the proof of concept behaves here in v5.0.1 (select HD and fullscreen for the best quality):

http://www.youtube.com/watch?v=hPy2Ivq3MxQ - Windows 7 SP1, the key was imported before recording the video.
http://www.youtube.com/watch?v=I96N-iZJoe8 - Windows XP SP3, shows the full process including the key import. This works a little bit slower due to a combination of video capturing and running things in a VM.

Enter was released as soon as the counter froze completely.
Comment 14 Jesse Ruderman 2011-07-20 02:50:05 PDT
> * embed pluginspage= can bypass the XPI whitelist (an annoyance hole, but
> not strictly a security bug)

I filed this as a security bug a few months ago ;) Bug 655916.

> Getting the browser into a state in which Services.ww is permanently undefined

Jeepers. This is why we need separate recursion limits for chrome & content.  And why we need to NS_RUNTIMEABORT when chrome js hits OOM or TMR -- we can't possibly audit our chrome js against the possibility that at any bytecode instruction the script could just stop executing.
Comment 15 Dave Townsend [:mossop] 2011-07-20 08:01:20 PDT
(In reply to comment #13)
> Dave, what version did you check? The proof of concept is suited for the
> current public release (5.0.1).

I was testing the latest nightly builds.
Comment 16 Dave Townsend [:mossop] 2011-07-20 08:02:57 PDT
(In reply to comment #14)
> > Getting the browser into a state in which Services.ww is permanently undefined
> 
> Jeepers. This is why we need separate recursion limits for chrome & content.
> And why we need to NS_RUNTIMEABORT when chrome js hits OOM or TMR -- we
> can't possibly audit our chrome js against the possibility that at any
> bytecode instruction the script could just stop executing.

I think the key for me is that, yes we could amend amWebInstallListener.js to behave in the case where Services.ww is undefined, but if things like that can be undefined then code all over the place is going to break. We should fix whatever is causing that.
Comment 18 Mariusz Mlynski 2011-07-21 21:14:13 PDT
(In reply to comment #15)
> I was testing the latest nightly builds.

I've checked the latest nightly build and I think the reason why you hadn't seen any windows while holding Enter is as follows:

In Firefox 5.0.1, the Embed element for an unsupported plugin looks like this: http://i.imgur.com/FTlKX.png -- the entire box links to Plugin Finder Service, it's also sufficient to focus the Embed element to get PFS windows while just holding Enter.

Nightly (8.0a1 2011-07-21) implements this feature as shown here: http://i.imgur.com/BiVtr.png -- the link is nested deep inside the XUL box, that's why focusing the Embed element isn't sufficient. However, it's possible to obtain a reference to the link if the user clicks within the boundaries of the XUL box (essentially anywhere on the page, as you can follow the cursor with the XUL element). The link can be focused subsequently. I will attach a simple test that shows how it's done.

Btw, from what I'm seeing, Services.ww can end up in the undefined state in the Nightly build (ie. the implications of a successful attempt are exactly the same), although there are more failures (broken windows after the try-catch recursion) where 5.0.1 would have a 100% success rate. It looks that the exploit works more reliably with the release build.
Comment 19 Mariusz Mlynski 2011-07-21 21:18:10 PDT
Created attachment 547601 [details]
Test focusing pluginspage install link in Nightly build.
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2011-08-04 13:44:17 PDT
Dave, is this something you can own? If not, can you help find an appropriate owner here? Thanks!
Comment 21 Dave Townsend [:mossop] 2011-08-04 13:52:28 PDT
Blair could you put together the simple fix here? Just wrap the Services.ww call in a try...catch and if it fails cancel the installs. No idea how you'd test that though
Comment 22 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-04 19:40:25 PDT
Created attachment 550934 [details] [diff] [review]
Patch v1

This fixes up the Services.ww issue, and tests it (test fails without the fix). As a bonus, it applies cleanly to the various branches.
Comment 23 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-10 19:39:36 PDT
https://hg.mozilla.org/mozilla-central/rev/84ce41f8cec7
Comment 24 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-11 00:41:36 PDT
Backed out due to a leak (that I don't yet understand)
https://hg.mozilla.org/mozilla-central/rev/aa9f527a4928
Comment 25 Johnny Stenback (:jst, jst@mozilla.com) 2011-08-18 13:57:00 PDT
Blair, any updates here? Need help with tracking this down, or you think you're making progress here?
Comment 26 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-18 23:03:57 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #25)
> Blair, any updates here? Need help with tracking this down, or you think
> you're making progress here?

Ugh, sorry, I've been struck down with the flu for a decent amount of the week. I could indeed use some help trying to figure out the leak - I'm kinda stumped.
Comment 27 Johnny Stenback (:jst, jst@mozilla.com) 2011-08-19 13:21:12 PDT
Andrew, could you help Blair look into the leak that the patch in this bug introduced?
Comment 28 Andrew McCreight [:mccr8] 2011-08-19 13:39:59 PDT
What test causes the leak?  General browsing, the test you added, something else?
Comment 29 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-21 20:22:41 PDT
(In reply to Andrew McCreight [:mccr8] from comment #28)
> What test causes the leak?

The test added in this patch.
Comment 30 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-22 00:08:54 PDT
I appear to have fixed the leak, though I don't understand how (but I'd like to!). This leaked:

delete Services.ww;
... do stuff ...
Services.ww = Cc["..."].getService(Ci.nsIWindowWatcher);


This doesn't:

var gWindowWatcher = Services.ww;
delete Services.ww;
... do stuff ...
Services.ww = gWindowWatcher;



However... I found another issue, where it was sending the addon-install-failed notification with an empty array of addons. And fixing that broke a bunch of assumptions the addons manager (and it's tests) make about how installs are expected to fail. Working on a new patch.
Comment 31 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-22 20:16:32 PDT
Created attachment 555021 [details] [diff] [review]
Patch v1.1

Waiting on Try results, just because I'm paranoid. But testing locally, the leak is gone, and the tests pass.

I looked into making the install actually fail (rather than just be canceled), but it would require adding a new API.
Comment 32 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-22 21:57:04 PDT
Comment on attachment 555021 [details] [diff] [review]
Patch v1.1

Scratch that. Try says it still leaks :(
Comment 33 Andrew McCreight [:mccr8] 2011-08-23 11:55:06 PDT
I can reproduce this locally with the first version of the patch, but not the second, so I looked at the first version to see if that might help.  I used the steps described in 
   https://bugzilla.mozilla.org/show_bug.cgi?id=561359#c60
There are two nsGlobalWindows being kept alive in the cycle collector via ChromeWindows:

0x11a913c58 [JS Object (ChromeWindow) (global=11a913c58)]
        --[xpc_GetJSPrivate(obj)]-> 0x11ac15ca0 [XPCWrappedNative (ChromeWindow)]
        --[mIdentity]-> 0x11ac14748 [nsGlobalWindow]

0x1238a1e50 [JS Object (ChromeWindow) (global=1238a1e50)]
        --[xpc_GetJSPrivate(obj)]-> 0x11bd43080 [XPCWrappedNative (ChromeWindow)]
        --[mIdentity]-> 0x11bd41f38 [nsGlobalWindow]
These ChromeWindows have been marked as reachable by the JS GC.

Looking at the GC dump, one of the ChromeWindows (0x11a913c58) seems to be kept alive because it is on the machine stack.  I only have one path for the other ChromeWindow, which says that it is being held alive via the first ChromeWindow, but there may be another path, too.

I think being stored on the machine stack means it is being kept in a local variable somehow, but I'm not sure.  I'll see if I can get more information about how that is happening, but given that it uses a conservative stack scanner I am not too hopeful.

Maybe this will help...
Comment 34 Andrew McCreight [:mccr8] 2011-08-23 15:38:29 PDT
The leak didn't go away when I undid the changes in amWebInstallListener.js, so I guess it is a problem with the test itself?
Comment 35 Andrew McCreight [:mccr8] 2011-08-23 18:27:14 PDT
Dao has a lot of experience with fixing tests that leak windows, maybe he can help here.
Comment 36 Dão Gottwald [:dao] 2011-08-24 01:35:21 PDT
I don't know about the leak (xpinstall tests are somewhat hard to follow because of the custom "Harness"), but I'd like to point out that messing with Services.ww like the patch does isn't nice at all, since you're sharing the browser with all other browser-chrome tests. E.g. if some chrome code runs simultaneously with your test and breaks, this might cause issues for all subsequent tests.
Comment 37 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-24 02:30:21 PDT
(In reply to Andrew McCreight [:mccr8] from comment #34)
> The leak didn't go away when I undid the changes in amWebInstallListener.js,
> so I guess it is a problem with the test itself?

AFAICT, yea. Which confuses me more, since it's a simple test using a harness that's already used by many other tests, without leaks.


(In reply to Dão Gottwald [:dao] from comment #36)
> but I'd like to point out that messing
> with Services.ww like the patch does isn't nice at all, since you're sharing
> the browser with all other browser-chrome tests. E.g. if some chrome code
> runs simultaneously with your test and breaks, this might cause issues for
> all subsequent tests.

I know. But that's the whole point of the test. So the alternative is to not have the test (which would make solving the leak a lot easier...).
Comment 38 Dão Gottwald [:dao] 2011-08-24 03:12:03 PDT
(In reply to Blair McBride (:Unfocused) from comment #37)
> (In reply to Dão Gottwald [:dao] from comment #36)
> > but I'd like to point out that messing
> > with Services.ww like the patch does isn't nice at all, since you're sharing
> > the browser with all other browser-chrome tests. E.g. if some chrome code
> > runs simultaneously with your test and breaks, this might cause issues for
> > all subsequent tests.
> 
> I know. But that's the whole point of the test.

Affecting random other code isn't point of the test...

> So the alternative is to not
> have the test (which would make solving the leak a lot easier...).

If you're confident that the leak isn't caused by your code, sure. You should probably file a bug on the leak, though, in case this issue already exists and your test just unveils it.
Comment 39 Johnny Stenback (:jst, jst@mozilla.com) 2011-08-25 13:38:54 PDT
If we believe that the fix for this is good and doesn't leak by itself, could we take the fix for this security bug w/o the leaking test to get this security fix out to our users sooner?
Comment 40 Dave Townsend [:mossop] 2011-08-26 14:15:56 PDT
Comment on attachment 555021 [details] [diff] [review]
Patch v1.1

r+ to land without the testcase
Comment 41 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-26 14:30:30 PDT
Landed sans test:
https://hg.mozilla.org/mozilla-central/rev/b379d2c614d0

And filed bug 682410 to figure out what's going on with that leak.
Comment 42 Mariusz Mlynski 2011-08-26 15:05:51 PDT
As a side note, the other issue highlighted in the exploit (non-whitelisted site can trigger xpinstall from pluginspage due to a ChromeWindow opener) is not resolved yet. With amWebInstallListener.js fixed by Blair to stop when xpinstallConfirm.xul fails to open, it shouldn't be possible any more to use it to do anything scary (at least without user's consent), but it may be worth fixing.
Comment 43 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-31 23:15:53 PDT
jst / Mossop: What say you to comment 42?
Comment 44 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-08-31 23:17:26 PDT
Comment on attachment 555021 [details] [diff] [review]
Patch v1.1

This didn't explode.
Comment 45 Johnny Stenback (:jst, jst@mozilla.com) 2011-09-01 13:26:00 PDT
Can we get this landed on m-c asap? We're running out of time for 7 here, and this needs to land on m-c before we approve it for branches.
Comment 46 Andrew McCreight [:mccr8] 2011-09-01 13:29:28 PDT
According to comment 41, it is on M-C, just sans test.
Comment 47 christian 2011-09-06 14:21:53 PDT
Comment on attachment 555021 [details] [diff] [review]
Patch v1.1

Denied for mozilla-beta. We'll let it bubble up from mozilla-aurora naturally. Please renominate if you disagree.
Comment 48 Daniel Veditz [:dveditz] 2011-09-06 14:33:30 PDT
Comment on attachment 555021 [details] [diff] [review]
Patch v1.1

This is essentially a drive-by install (a little bit of easily lured user interaction). Why in the world would we not take this simple patch to prevent that? Patch and testcase checked into trunk/aurora raises the risk of someone else figuring this out.

Renominating for Firefox 7 beta
Comment 49 christian 2011-09-06 14:48:46 PDT
Comment on attachment 555021 [details] [diff] [review]
Patch v1.1

On further review we will take this on mozilla-beta. Please land asap.
Comment 50 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-09-06 21:02:02 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/cb0f5868c9e0
https://hg.mozilla.org/releases/mozilla-beta/rev/51780cb829df
Comment 51 juan becerra [:juanb] 2011-09-09 16:13:30 PDT
QA tracking this for verification across branches.
Comment 52 Daniel Veditz [:dveditz] 2011-09-18 10:01:27 PDT
Not sure if this applies to Firefox 3.6.x or not. The patch doesn't, there's no equivalent use of Services.ww in the old code. I don't think anything was done here about the reporters first point in comment 0, but I did see that effect in any of the testcases attached or linked-to by this bug.
Comment 53 Mariusz Mlynski 2011-09-18 13:48:44 PDT
(In reply to Daniel Veditz from comment #52)
> Not sure if this applies to Firefox 3.6.x or not.

It doesn't, the old code defines its own window watcher:

var ww = Cc["@mozilla.org/embedcomp/window-watcher;1"].getService(Ci.nsIWindowWatcher);
ww.openWindow(null, URI_XPINSTALL_CONFIRM_DIALOG,"", "chrome,centerscreen,modal,dialog,titlebar", ifptr);
if (!dpb.GetInt(0)) {// User said OK - install items

I see no way it could fail and proceed with the installation at the same time.
Comment 54 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-26 09:54:59 PDT
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0) Gecko/20100101 Firefox/7.0 ID:20110922153450

I'm not sure if this is fixed or if I am testing this wrong. Here is what I did:

1) Import the attached cert
2) Close Firefox and restart after the process is closed
3) Go the the website in comment 1 (404'd so I'm guessing this is broken)
4) Go to attached Proof-of-Concept
5) Click the button on the page
6) Press ENTER key

Result:
Plugin installer dialog appears

Am I doing this wrong or is this not fixed on Firefox 7?
Comment 55 Mariusz Mlynski 2011-09-26 17:11:58 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #54)
> 3) Go the the website in comment 1 (404'd so I'm guessing this is broken)

It's up and running (a slash was missing, see comment 5), although it didn't take account of the different "missing plugin" XUL box in the newer versions of Firefox (see comment 18). I've now repacked signed.jar to include a new file exploitn.htm, which uses the pluginspage link focus method from attachment 547601 [details], so you can just replace "signed.jar!/exploit.htm" with "signed.jar!/exploitn.htm" and proceed as described if you're not using Firefox 5.

Btw, check out the last paragraph in comment 18. If it happens to throw corrupt windows after the alert dialog, kill the process and retry.

> 6) Press ENTER key

And hold it down until the counter freezes.

Firefox 7.0b6, Windows 7 SP1, using a fairly fresh user profile: http://www.youtube.com/watch?v=044-OTl0TZw (xpinstallConfirm.xul fails to open, but the extension is not installed)
Comment 56 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-30 16:32:06 PDT
Website is still 404 for me...
Comment 57 Mariusz Mlynski 2011-09-30 16:49:08 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #56)
> Website is still 404 for me...

I've checked the logs and the last link you were trying to access (with a slash after "!" and "exploitn.htm" as the internal filename) was correct, you just missed the "jar:" prefix before "http:", hence 404.
Comment 58 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-30 17:10:22 PDT
Created attachment 563900 [details]
Screenshot: Test

Okay, I reran the steps with the correct URL and here is my result.

Firefox 9.0a2 on Ubuntu 11.04 64-bit. Is this expected given the bug is "resolved"?
Comment 59 Mariusz Mlynski 2011-09-30 17:40:54 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #58)
> Firefox 9.0a2 on Ubuntu 11.04 64-bit. Is this expected given the bug is
> "resolved"?

Yes, it seems that the trick to keep Services.ww unregistered failed, so there's no sign of any incorrect behaviour (except the thing mentioned in comment 42: the ChromeWindow opener allows the confirm dialog to appear without asking), thus it's all good. It may well be that clobbering enablePrivilege in the callback to keep ww from being defined is a Windows-specific thing -- I've never tested it on Unix/Mac, and on Windows in the most recent Firefox versions it's either a complete failure or a complete success (up to the point of Blair's fix, which silently cancels the installation).
Comment 60 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-30 17:43:48 PDT
So does that mean this bug can be marked VERIFIED FIXED?
Comment 61 Mariusz Mlynski 2011-09-30 17:49:02 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #60)
> So does that mean this bug can be marked VERIFIED FIXED?

Yes, I think so, there's no way it can be exploited in FF7+, and the problem described in comment 42 will be resolved with the fix to bug 655916.
Comment 62 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-30 17:52:26 PDT
Okay, marking verified. I confirm the same behaviour in Firefox 7.0, 8.0b1, and 9.0a2.

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