990 bytes, patch
|Details | Diff | Splinter Review|
3.87 KB, patch
|Details | Diff | Splinter Review|
See Also: → CVE-2014-1510
Alias: ZDI-CAN-2215 → CVE-ZDI-1511
Summary: popup blocker bypass → popup blocker bypass (ZDI-CAN-2215)
I looked at this briefly and this is the most obvious patch around. I don't see any good reasons to skip doing the security check, whether or not our current context is related to a window or not. The free pass patch goes all the way back to the aviary merge and simple investigation didn't turn up the original bug that it was added for. There might be a better fix to be had...
Comment on attachment 8390163 [details] [diff] [review] Most obvious patch Sorry, attached this to the wrong bug.
(The patch was intended for bug 982906, and is now there.)
This fixes the testcases (on linux they just throw when they can't access the Windows specific files). Going through still other possible similar stuff, but we don't have other OpenJS C++ callers.
Comment on attachment 8390177 [details] [diff] [review] obvious patch Many thanks to Gabor for cleaning up the GetWindow/GetInnerWindow mess enough to make this patch correct.
Attachment #8390177 - Flags: review?(mrbkap) → review+
Hmm, that patch is for trunk. Testing on Aurora and Beta...
Aurora and Beta should be ok. Looking esr...
Aha, ESR needs more checks.
Comment on attachment 8390177 [details] [diff] [review] obvious patch r=jst for this, I'll continue thinking about other places/approaches to this problem...
Attachment #8390177 - Flags: review?(jst) → review+
Hmm, on esr24 showModalDialog may have similar problem.
Distrust all ancient hacks that lack any rationale! Any more like this? Code smells from light years away. Sorry I didn't notice from my quadrant :-|. /be
Ah, the fix is actually enough in esr24 case too, since GetInnerWindow does innerwindow correctness check. But ShowModalDialog needs a fix on esr24, as far as I see. (Don't have a testcase for it though.)
A bit hackish macro. We get to those checks from inner window, and then forward to outer, so we're really interested in only the inner window case. Newer branches have similar checks in FORWARD_TO_OUTER, but adding similar check for all the cases in esr24 feels tiny bit risky. The reason for the macro is that showModalDialog looks scary. It does FORWARD_TO_OUTER without any security checks, so OpenInternal ends up dealing with outer window, just like in OpenJS case of HTMLDocument::Open.
Attachment #8390265 - Flags: review?(jst)
Paul - Is this needed for FxOS 1.3? Please nom if it's needed.
I'm not sure how to test if this affects b2g or not, but from my naive understanding of this bug, I can't see why it wouldn't affect b2g, so requesting 1.3+. The issue of modifying the frame so that browser.js  retrieves data from the frame and assigns it to a src obviously wont apply since b2g doesn't have xul. But there may be similar situations in b2g chrome code elsewhere.  http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#843
Comment on attachment 8390177 [details] [diff] [review] obvious patch [Security approval request comment] How easily could an exploit be constructed based on the patch? Not easily, but this is pwn2own Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Which older supported branches are affected by this flaw? All Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? See the patch for esr24. The fix for esr24 is actually exactly the same, but showModalDialog handling looks similarly scary there so additional check is added. (don't have a test to show that showModalDialog actually would have the same issue) How likely is this patch to cause regressions; how much testing does it need? This should be quite safe. If document doesn't have inner window at all, it sure is gone, and the other check in FORWARD_TO_OUTER lets document.open to work even after document.write (document.write creates a new inner window), but not after a new page load.
Attachment #8390265 - Flags: approval-mozilla-esr24?
smaug, I think the beta approval request should technically be a release approval request, as 28 is in the release repo already. Release Management will see it as that anyhow, I guess.
Well, current beta is 28, so I'm asking approval for that, not to the current release build. If we had a 0-day, I'd ask approval for the release.
Or at least afaik, this isn't actually 0-day. This bug isn't being used actively atm, and we weren't going to put these fixes to FF27. (But I guess it depends on the definition of a 0-day)
Comment on attachment 8390177 [details] [diff] [review] obvious patch sec-approval+ for trunk and aurora+.
Attachment #8390265 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
Comment on attachment 8390177 [details] [diff] [review] obvious patch [Triage Comment] Landing this to mozilla-beta just for sanity, but we'll actually want this on mozilla-release so as to get it in the 28 respin of final RC -- also approving the esr 24 patch for uplift so it's in the 24.4.0 esr that ships alongside 28
I don't see a reporter name for this. Is this Mariusz, Curstis?
(In reply to Al Billings [:abillings] from comment #26) > I don't see a reporter name for this. Is this Mariusz, Curstis? Yes, this and bug 982906 were both used in concert by Mariusz for his attack.
Whiteboard: [pwn2own 2014] → [pwn2own 2014][adv-main28+]
Olli, what should the rating be here? Inner-outer window confusion on nsHTMLDocument::Open sounds much worse than just a popup blocker bypass. Does sec-critical make sense, or maybe sec-high or something else?
Well, this + bug 982906 is critical, so I think they both should be handled as critical.
https://hg.mozilla.org/mozilla-central/rev/e522b5f583ee https://hg.mozilla.org/releases/mozilla-aurora/rev/1b0404fd3518 https://hg.mozilla.org/releases/mozilla-beta/rev/82b2949b1ec2 https://hg.mozilla.org/releases/mozilla-release/rev/82b2949b1ec2 https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/82b2949b1ec2 https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/30c60b474918 https://hg.mozilla.org/releases/mozilla-esr24/rev/524b8c4cd680 https://hg.mozilla.org/releases/mozilla-b2g18/rev/75bdbb5f07a2 https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/75bdbb5f07a2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Verified as fixed with the 03/14 Aurora and Nightly on Windows 8.1 64bit, Mac OS X 10.8.5 and Ubuntu 13.04 32bit. No pop-ups are opened by the testcases attached to this bug.
tracking-fennec: --- → ?
Confirmed bug on Fx27.0.1, Windows 8. Verified fix on Fx28, Fx24.4.0esr.
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.