Bug 552255 (CVE-2010-1125)

focus() behavior may be used to inject, maybe steal keystrokes

RESOLVED FIXED

Status

()

Core
Event Handling
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Michal Zalewski, Assigned: smaug)

Tracking

(Blocks: 1 bug, {verified1.9.2})

1.9.2 Branch
verified1.9.2
Points:
---

Firefox Tracking Flags

(blocking1.9.2 needed, status1.9.2 .4-fixed, status1.9.1 unaffected)

Details

(Whiteboard: [sg:moderate])

Attachments

(3 attachments)

(Reporter)

Description

7 years ago
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.
Component: Security → Event Handling
Product: Firefox → Core
QA Contact: firefox → events
Version: 3.6 Branch → 1.9.2 Branch
(Assignee)

Comment 1

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

Comment 2

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

Updated

7 years ago
Blocks: 457011
Whiteboard: [sg:moderate][do not disclose until webkit is also fixed]
(Assignee)

Comment 3

7 years ago
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.
Assignee: nobody → Olli.Pettay
(Assignee)

Comment 4

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

Comment 5

7 years ago
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).
Attachment #433110 - Flags: feedback?(enndeakin)
(Reporter)

Comment 6

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

Comment 7

7 years ago
Ok, thanks for clarification.

The patch should fix the first test case, and yet it doesn't break 
tabbing through iframes.

Comment 8

7 years ago
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?
Attachment #433110 - Flags: feedback?(enndeakin) → feedback+
(Assignee)

Comment 9

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

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

Comment 11

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

Comment 12

7 years ago
I think the two blocks work quite well together. They are not for the same thing.
(Assignee)

Comment 13

7 years ago
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
Attachment #433110 - Flags: review?(enndeakin)
Alias: CVE-2010-1125

Comment 14

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

Comment 15

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

Updated

7 years ago
Attachment #433110 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 16

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

Comment 17

7 years ago
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.
Attachment #433110 - Flags: superreview?(jst)

Updated

7 years ago
Attachment #433110 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 18

7 years ago
Michal, Reed, do you think it would be possible to get the original
testcase somewhere.
blocking1.9.2: --- → ?
blocking2.0: --- → ?
(Reporter)

Comment 19

7 years ago
Sorry for the trouble. My homepage is now up.
blocking1.9.2: ? → needed
(Assignee)

Comment 20

7 years ago
Ok, I can't reproduce the bug on 3.5. (1.9.1 branch)
(Assignee)

Comment 21

7 years ago
Created attachment 440774 [details] [diff] [review]
the same for 1.9.2
(Assignee)

Comment 22

7 years ago
http://hg.mozilla.org/mozilla-central/rev/bb175d10ce90

Waiting for some days before asking approval for 1.9.2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Reporter)

Comment 23

7 years ago
When do you expect this to be approved & released?
(Assignee)

Comment 24

7 years ago
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.
Attachment #440774 - Flags: approval1.9.2.5?
Attachment #440774 - Flags: approval1.9.2.4?
(Assignee)

Comment 25

7 years ago
I don't know the timeframe
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
Attachment #440774 - Flags: approval1.9.2.4? → approval1.9.2.4-
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 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
Attachment #440774 - Flags: approval1.9.2.5?
Attachment #440774 - Flags: approval1.9.2.5-
Attachment #440774 - Flags: approval1.9.2.4-
Attachment #440774 - Flags: approval1.9.2.4+
(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?
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/99846558f1fb
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5c2487b51933
blocking2.0: ? → ---
status1.9.2: --- → .4-fixed
(Reporter)

Comment 31

7 years ago
The original PoC definitely works on 3.5 on Windows, not sure if that's what you mean.
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?
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.
If someone can reproduce this on 1.9.1.3, please remove the unaffected and request blocking.
status1.9.1: --- → unaffected
(Reporter)

Comment 35

7 years ago
Arrgh, actually, true. Sorry for the confusion. 3.5.x looks OK.
status1.9.1: unaffected → ---
(Reporter)

Comment 36

7 years ago
...and fixing edit collision.
status1.9.1: --- → unaffected

Comment 37

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

Comment 39

7 years ago
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.
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).
Keywords: verified1.9.2
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.
(Reporter)

Comment 42

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

Comment 43

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

Comment 44

7 years ago
Apple and everybody else just went public with this. When do you plan to ship 3.6.4?

Comment 45

7 years ago
"Early June" (https://wiki.mozilla.org/Releases). Likely in the coming week, though that is not set in stone.
Group: core-security
Whiteboard: [sg:moderate][do not disclose until webkit is also fixed] → [sg:moderate]

Comment 46

7 years ago
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
You need to log in before you can comment on or make changes to this bug.