Last Comment Bug 552255 - (CVE-2010-1125) focus() behavior may be used to inject, maybe steal keystrokes
(CVE-2010-1125)
: focus() behavior may be used to inject, maybe steal keystrokes
Status: RESOLVED FIXED
[sg:moderate]
: verified1.9.2
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: 1.9.2 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug] (high review load, please consider other reviewers)
:
Mentors:
Depends on:
Blocks: clickjacking
  Show dependency treegraph
 
Reported: 2010-03-14 00:47 PST by Michal Zalewski
Modified: 2010-07-22 00:19 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
needed
.4-fixed
unaffected


Attachments
a patch (6.71 KB, patch)
2010-03-17 11:39 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
enndeakin: review+
jst: superreview+
enndeakin: feedback+
Details | Diff | Review
a testcase (451 bytes, text/plain)
2010-03-29 07:35 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
no flags Details
the same for 1.9.2 (6.74 KB, patch)
2010-04-22 08:42 PDT, Olli Pettay [:smaug] (high review load, please consider other reviewers)
mbeltzner: approval1.9.2.4+
mbeltzner: approval1.9.2.5-
Details | Diff | Review

Description Michal Zalewski 2010-03-14 00:47:47 PST
Hi folks,

I originally thought this only applies to WebKit, but realized it can be tweaked to work with Firefox only after making this proof-of-concept public. Sorry for that - it's fairly low risk, but probably interesting:

http://lcamtuf.coredump.cx/focus-webkit/

This PoC is probably Windows-specific. Unlike clickjacking, this permits a fairly high degree of sophistication in interacting with third-party sites.

An obvious way to mitigate the risk is to mimic the behavior of MSIE / Opera - that is:

1) Forbid or defer focus changes between onkeydown and onkeyup, so that a malicious page can't see what's being pressed, but deliver the result elsewhere,

2) When focus is grabbed by another document in an IFRAME, and then returned, we should probably not attempt to restore it to the last known editing location, but reset it instead.

Now, the more interesting part. NOTE: THIS PART OF THE BUG IS NOT PUBLIC. We also noticed that the following is possible in WebKit browsers, and to a more limited extent and with some tweaks, in MSIE:

http://lcamtuf.coredump.cx/focus-webkit/toplevel.html

In essence, the same attack can be reversed, and allow cross-domain IFRAME gadgets to steal keystrokes (including password field entries) transparently.

This attack does not work on Firefox as-is, but this seems to be due to a conjunction of what I presume are two bugs; and not a conscious security decision:

1) Calling top.focus() always seem to be silently ignored, even if the current browser window is, in fact, in focus, and the call is being made from a currently in-focus IFRAME on that page. In other browsers, this simply returns focus to the top-level document. In Firefox, it does not, presumably because of an overzealous anti-annoyance filter meant to prevent pop-ups from doing setInterval('focus()',1).

2) The other way to approach this would be to call <iframedwindow>.blur(), but counterintuitively, this blurs the entire browser window (and amusingly, doing setInterval('blur()',1) from within an IFRAME makes the browser unusable). This looks like something that may eventually get fixed by accident.

I suspect there might be some other way to restore focus to the top-level doc, but I have no obvious ideas. Even if not, it is a more compelling argument to implement the aforementioned changes to focus behavior.
Comment 1 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-14 09:23:56 PDT
(In reply to comment #0)
> 1) Forbid or defer focus changes between onkeydown and onkeyup, so that a
> malicious page can't see what's being pressed, but deliver the result
> elsewhere,
Need to be careful with a change like this. I believe some services
depend on changing focus between keydown and keyup if IME is involved.
Or something like that. I don't have any examples.

> 2) When focus is grabbed by another document in an IFRAME, and then returned,
> we should probably not attempt to restore it to the last known editing
> location, but reset it instead.
I believe when returning user does expect focus to be restored to the last known
editing location. What does "reset" even mean in this context?
But still, perhaps we could handle this so that if focus changes between
keydown and keyup, the focus wouldn't return to the original editing location.
Though, even that could break web apps.
Comment 2 Michal Zalewski 2010-03-14 10:31:50 PDT
The behaviors described are, as far as I can tell, already present in MSIE and Opera to some extent, so I am not very concerned about breakage (though testing would be prudent).
Comment 3 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-15 16:13:27 PDT
I am always *very* concerned about breakage when changing focus handling code.

Taking for investigation. I'll try to find time to look at this tomorrow.
Comment 4 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-16 15:27:01 PDT
My current plan is to prevent focusing not-same-origin window during user initiated key events. Or make that kind of focusing asynchronous.
Need to test both approaches.
Comment 5 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-17 11:39:49 PDT
Created attachment 433110 [details] [diff] [review]
a patch

This is one option.

I don't quite understand why the first testcase is really a security bug at all.
The second one is more serious and trickier, fortunately it doesn't apply to
gecko at the moment.

Need to test this on windows (tested only Linux).
Comment 6 Michal Zalewski 2010-03-17 11:52:02 PDT
The first test case is a problem similar to clickjacking:

http://code.google.com/p/browsersec/wiki/Part2#Arbitrary_page_mashups_(UI_redressing)

It allows UI actions to be carried out across domains in a matter that defies the usual cross-site request forgery defenses (http://en.wikipedia.org/wiki/XSRF). The provided example is obviously harmless, and since Google Search is not XSRF-protected, it could be achieved by just doing <iframe src="http://www.google.com/search?q=porn"></iframe>; but consider an XSRF-protected page on Facebook that changes your privacy settings, or adds someone as your friend, or deletes your account.
Comment 7 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-17 12:05:31 PDT
Ok, thanks for clarification.

The patch should fix the first test case, and yet it doesn't break 
tabbing through iframes.
Comment 8 Neil Deakin 2010-03-18 17:31:00 PDT
Comment on attachment 433110 [details] [diff] [review]
a patch

The approach looks ok.

>+    if (!focusedPrincipal || !newPrincipal) {
>+      return;
>+    }
>+    PRBool subsumes = PR_FALSE;
>+    focusedPrincipal->Subsumes(newPrincipal, &subsumes);

Don't you mean for the principals to be the other way around?
Comment 9 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-19 03:50:50 PDT
First I had it other way around, but then thinking for example the case
when focus is atm in chrome, it should be able to focus whatever it wants.

Or am I thinking wrong here.
Excluding the chrome case, if the pages aren't from same domain, subsumes
should be false either way.
Comment 10 Neil Deakin 2010-03-19 05:23:51 PDT
When focus is in chrome, the page should not be able to grab the focus. Although it can shift the currently focused element within the page, it shouldn't be able to shift the current window. That's already the case, see bug 125282 for that. Maybe the two code blocks could be combined as well.
Comment 11 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-19 05:35:12 PDT
In the first testcase it is the currently focused window which tries to
focus iframe. That is what I'm trying to prevent.

(I can't access the testcase atm.)
Comment 12 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-19 07:49:49 PDT
I think the two blocks work quite well together. They are not for the same thing.
Comment 13 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-19 14:14:57 PDT
Comment on attachment 433110 [details] [diff] [review]
a patch

Seems like I need to rewrite the testcase, since the original one isn't reliably available.

So Neil, do you want some changes to this.

Need to figure out a fix for 1.9.1.x
Comment 14 Neil Deakin 2010-03-29 07:04:13 PDT
(In reply to comment #11)
> In the first testcase it is the currently focused window which tries to
> focus iframe. That is what I'm trying to prevent.
> 
> (I can't access the testcase atm.)

I think I need to see the testcase again before I can understand what the issue is here. Is the security issue only relevant when the new frame is from a different source, and that switching to another frame that can be accessed during a key event is ok? If so, the patch looks ok.
Comment 15 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-29 07:35:25 PDT
Created attachment 435617 [details]
a testcase

Here is a testcase. This doesn't do the key event filtering the original
testcase was doing, but should still show where the problem is.
Comment 16 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-29 08:32:51 PDT
My testcase doesn't show the problem on 1.9.1.x and I wonder if
the original testcase worked there either.

Michal, any chance to get your testcases attached to this bug?
Comment 17 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-31 12:03:44 PDT
Comment on attachment 433110 [details] [diff] [review]
a patch

this needs sr.

I'm still hoping to get the original testcase online, so that I could verify
whether the patch fixes that also on 1.9.2 branch, and whether to fix something
on 1.9.1.x.
Comment 18 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-04-20 03:07:13 PDT
Michal, Reed, do you think it would be possible to get the original
testcase somewhere.
Comment 19 Michal Zalewski 2010-04-21 10:50:00 PDT
Sorry for the trouble. My homepage is now up.
Comment 20 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-04-22 07:56:46 PDT
Ok, I can't reproduce the bug on 3.5. (1.9.1 branch)
Comment 21 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-04-22 08:42:46 PDT
Created attachment 440774 [details] [diff] [review]
the same for 1.9.2
Comment 22 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-04-24 03:50:26 PDT
http://hg.mozilla.org/mozilla-central/rev/bb175d10ce90

Waiting for some days before asking approval for 1.9.2
Comment 23 Michal Zalewski 2010-04-28 08:58:25 PDT
When do you expect this to be approved & released?
Comment 24 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-04-28 09:18:43 PDT
Comment on attachment 440774 [details] [diff] [review]
the same for 1.9.2

I assume not 1.9.2.4, but hopefully 1.9.2.5.
Comment 25 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-04-28 09:18:59 PDT
I don't know the timeframe
Comment 26 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-28 12:48:58 PDT
Comment on attachment 440774 [details] [diff] [review]
the same for 1.9.2

Too late to take this for Firefox 3.6.4, we'll get it for Firefox 3.6.5
Comment 27 Reed Loden [:reed] (use needinfo?) 2010-04-28 13:13:59 PDT
My understanding is that while this bug is security-sensitive, most of the issue itself is public (CVE-2010-1125) with a public testcase (see http://www.securityfocus.com/archive/1/archive/1/510070/100/0/threaded). There seems to be a few parts in comment #0 that aren't yet public, but MITRE felt this affected Firefox enough to assign it a CVE already, so if we can go ahead and get this into 1.9.2.4 which hasn't started building yet, it would be better than waiting another 6 weeks.
Comment 28 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-28 13:15:28 PDT
Comment on attachment 440774 [details] [diff] [review]
the same for 1.9.2

I agree with Reed; we need to respond with agility if this is disclosed and public

a=beltzner for 1.9.2, please land on default and GECKO1924_20100413_RELBRANCH
Comment 29 Al Billings [:abillings] 2010-04-28 13:22:00 PDT
(In reply to comment #20)
> Ok, I can't reproduce the bug on 3.5. (1.9.1 branch)

We're sure that this won't affect 1.9.1?
Comment 31 Michal Zalewski 2010-04-28 13:51:02 PDT
The original PoC definitely works on 3.5 on Windows, not sure if that's what you mean.
Comment 32 Al Billings [:abillings] 2010-04-28 14:33:10 PDT
The original PoC works on 1.9.1 but we don't think we need to patch it on 1.9.1? Can someone explain this with clarity?
Comment 33 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-28 15:59:56 PDT
I can't get the original PoC to work on Firefox 3.5.9 (mozilla-1.9.1) under Windows.

In Firefox 3.6.3 it captures some (but not all) text.
Comment 34 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-28 16:02:55 PDT
If someone can reproduce this on 1.9.1.3, please remove the unaffected and request blocking.
Comment 35 Michal Zalewski 2010-04-28 16:06:03 PDT
Arrgh, actually, true. Sorry for the confusion. 3.5.x looks OK.
Comment 36 Michal Zalewski 2010-04-28 16:11:03 PDT
...and fixing edit collision.
Comment 37 christian 2010-04-28 17:26:27 PDT
Am I missing something? When I go to http://lcamtuf.coredump.cx/focus-webkit/toplevel.html with 3.5.9 on Win7 and type "Whatever" in the search box it instead goes into the evil gadget. Doesn't that mean 1.9.1 is affected?
Comment 38 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-28 17:44:34 PDT
I was referring to the PoC at http://lcamtuf.coredump.ca/focus-webkit - the more complicated case you refer to seems to do strange things on Firefox 3.5.x, but jumps cursor focus out of the box, which isn't really keystroke-stealing, it's focus-stealing :)
Comment 39 Michal Zalewski 2010-04-28 18:16:06 PDT
Yeah, the second test case does not work as-is, but this seems to be because of unrelated focus() / blur() quirks that will probably get fixed at some point (see my original message), so looks like it's worth keeping in mind anyway.
Comment 40 Al Billings [:abillings] 2010-04-30 16:08:01 PDT
Verified for 1.9.2 using the PoC and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.5pre) Gecko/20100429 Namoroka/3.6.5pre (.NET CLR 3.5.30729).
Comment 41 Brandon Sterne (:bsterne) 2010-05-04 16:48:51 PDT
Michal, this is fixed in the next shipping version of Firefox 3.6.  Has the issue been fixed in WebKit yet?  I'm trying to coordinate our advisories and need to know if this issue has to remain embargoed.  Thanks.
Comment 42 Michal Zalewski 2010-05-04 17:09:13 PDT
WebKit is still thinking. I think it's OK to mention this in release notes, just not unlock this bug. I will update this bug as soon as they're done.
Comment 43 Michal Zalewski 2010-05-05 11:26:53 PDT
OK, WebKit is done:

https://bugs.webkit.org/show_bug.cgi?id=26824
http://trac.webkit.org/changeset/58829

Given the fact it's moderate risk, I suspect it's going to be several months for all the vendors to pick it up, though; I will probably discuss the additional details before then, given the low risk of things going wrong.
Comment 44 Michal Zalewski 2010-06-08 13:49:33 PDT
Apple and everybody else just went public with this. When do you plan to ship 3.6.4?
Comment 45 christian 2010-06-09 10:03:43 PDT
"Early June" (https://wiki.mozilla.org/Releases). Likely in the coming week, though that is not set in stone.
Comment 46 Sasha (retired) 2010-07-22 00:19:19 PDT
Why use JS at all: http://pastie.org/1054910 and with HTML5's Boolean attribute 'autofocus' it becomes even easier to create a script-less keystroke stealer: http://pastie.org/1054917

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