This is a regression from bug 918345. The new implementation of nsGlobalWindow::OpenDialog doesn't check if it's called from chrome, which allows web content to open privileged windows. The attached example launches C:\Windows\System32\calc.exe (Windows) or alerts Components.classes (other systems) in Nightly 27.0a1 (2013-10-28).

Comment on attachment 823355 [details] [diff] [review] Patch (v1) [Security approval request comment] How easily could an exploit be constructed based on the patch? The patch makes it clear that this is a chrome permission check, and hence exposes the bug, but coming up with the exploit in the test case here probably takes a bit more effort. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? See above. Which older supported branches are affected by this flaw? Firefox 27, where bug 918345 landed. If not all supported branches, which bug introduced the flaw? Bug 918345. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? N/A. How likely is this patch to cause regressions; how much testing does it need? It should bring us back to the old behavior, so the risk for regressions should be minimal.

Comment on attachment 823355 [details] [diff] [review] Patch (v1) Did you try the patch? I don't see how this could fix it, this is a non-scriptable method.

We probably just need to mark openDialog ChromeOnly in the Window.webidl.

Working on an automated testcase, pretty sad that we didn't have one already.

Marking this as [ChromeOnly] will hide the method from regular content, which changes our existing behavior. Is that OK?

Calling it has always thrown so you'll just get a different kind of exception (ReferenceError vs a security exception). The only difference in behaviour is if it's used for browser detection ("openDialog in window"), but I think we should try it anyway.

And actually, until we switch completely to WebIDL for the Window object there won't be a difference at all, since we'll just fall back to XPConnect if the WebIDL property isn't installed.

Comment on attachment 823504 [details] [diff] [review] v1 r=me

[Security approval request comment] How easily could an exploit be constructed based on the patch? Fairly easily. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Yeah, we're specifically marking an accidentally exposed API as ChromeOnly. Which older supported branches are affected by this flaw? Firefox 27. If not all supported branches, which bug introduced the flaw? Bug 918345. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The patch will be identical. How likely is this patch to cause regressions; how much testing does it need? Very low risk, we're basically reverting to the previous behaviour.

If we haven't branched to make 27 Aurora and Central into 28 (effectively) yet, you can check this in without sec-approval. I thought the branching happened later today. Otherwise, I'll give it sec-approval but we'll have to put it on Aurora if we go post-branching.

Comment on attachment 823577 [details] [diff] [review] v1 with testcase Looks like we just missed the branching. I'm giving sec-approval+ and approving for Aurora to get this in.

setting status-firefox28 to fixed per comment #16

Confirmed issue on FF27, 2013-10-28. Verified fixed, FF27 2013-11-20. Verified fixed, FF28 2013-11-26.