370092 - (CVE-2006-2894) Focus change between onKeyDown and onKeyPress, allowing to read arbitary files using <input type=file> (Zalewski Firefox focus stealing vulnerability)

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Ben Bucksch (:BenB)]

Focus change allowed between onKeyDown and onKeyPress,
allowing attacker to read arbitary files,
abusing <input type=file> with user input

From: Michal Zalewski <[e-mail]>
To: [e-mail], [e-mail]
Date: Sun, 11 Feb 2007 21:00:45 +0100 (CET)
Message-ID: <Pine.LNX.4.58.0702112047050.30989@dione>

There is an interesting logic flaw in Mozilla Firefox web browser.

The vulnerability allows the attacker to silently redirect focus of
selected key press events to an otherwise protected file upload form
field. This is possible because of how onKeyDown / onKeyPress events are
handled, allowing the focus to be moved between the two. If exploited,
this enables the attacker to read arbitrary files on victim's system.

This was tested with 2.0.0.1. Opera is most likely not vulnerable;
Microsoft Internet Explorer is not vulnerable as-is, but might be
vulnerable to a variant of the attack.

All INPUT TYPE=FILE form fields enjoy the benefits of added protection to
prvent scripts from arbitrarily choosing local files to be uploaded to the
server, and automatically submitting the form. For example, .value
parameter cannot be set or changed, and any changes to .type reset the
contents of the field.

Unfortunately, Firefox allows a malicious script to redirect carefully
selected, individual user keystrokes to a hidden file upload field, in
order to compose a particular filename, then submit the form. User
interaction is required, limiting the impact somewhat - but any website
where the user can be reasonably expected to enter some text (a
keyboard-controlled web game, a blog posting or commenting interface) can
attempt to exploit the vulnerability, and eventually succeed with one user
or another.

A quick and naive demonstration of the problem (Firefox on Windows is
required;  depends on scancode values, so not all keyboards may be
supported):

  http://lcamtuf.coredump.cx/focusbug/

(Ta-dah again)

/mz

---

And indeed - here's a MSIE 7.0 demo:

http://lcamtuf.coredump.cx/focusbug/ieversion.html

Somewhat less pretty and straightforward, but equally sad.

/mz

---

pdp (architect) wrote:

> > here is an idea... we can combine both techniques into a single
> > attack... the hardest part of your hack is to force the user to type
> > :// plus several other /

Actually, MSIE doesn't require drive specification in the filename, and
will probably accept relative paths as well (so you might not need \
either when picking files from the desktop or 'my documents' or whatnot).

Firefox won't settle for a path without drive specification (but it will
accept SMB requests  ;-) . On *nix systems, of course, aiming /etc/passwd is
easier than C:\whatever.

The problem with intercepting address bar input is that you can't echo the
entered text back there without unloading the current document and its
scripts; in my examples, I tried to make sure that it's hard for the user
to notice that his input is not going where it should (in MSIE example,
this includes simulation of a blinking cursor).

---

[Similar is IE bug] MS00-093, but that's long fixed. [...]

/mz

---

On Sun, 11 Feb 2007, Michal Zalewski wrote:

> > http://lcamtuf.coredump.cx/focusbug/index.html (FF)
> > http://lcamtuf.coredump.cx/focusbug/ieversion.html (MSIE)

Paul Szabo pointed out that this is related to exploits posted by Charles
McAuley and Bart van Arnhem in June 2006 (CVE-2006-2894). These guys did
not demonstrate a complete attack, and their examples do not seem to work
well with MSIE 7 (which might be because of a half-assed attempt to fix
the problem, or perhaps not) - but a credit quite certainly is due.

Original postings:

  http://lists.grok.org.uk/pipermail/full-disclosure/2006-June/046610.html
  http://lists.grok.org.uk/pipermail/full-disclosure/2006-June/046699.html

Cheers,
/mz

Comment 1

[Boris Zbarsky [:bzbarsky]]

So this is branch-only, since on trunk we can't type into password inputs, right?

Could we somehow enforce that at textfield that's part of a password input only reacts to keypress if it got the keydown as well?  That seems like the right approach here...

Comment 2

[Michal Zalewski]

Boris - password inputs, or file inputs? Also, MSIE uses the policy you propose, but this is not a sufficient fix - a variant of this attack for MSIE7 selectively takes focus away from an off-screen file input field to perform the same attack, in a somewhat less glamorous manner.

This could work if you also prevent scripts from installing onkey* handlers on file inputs, and reading .value of such fields. 

The only other option I see is to simply prevent scripts from focusing on file input fields altogether.

Comment 3

[Ben Bucksch (:BenB)]

See also bug 370094 - can steal focus from addressbar.

Comment 4

[Ben Bucksch (:BenB)]

> since on trunk we can't type into password inputs, right?

You mean you can only select files via the file open dialog, not by typing paths?

---
Michal wrote:

And it's not really that much of an issue: disallow script-assisted
focusing on file input fields, or a) prevent event target from being
changed in onKeyDown (this is what MSIE does) + b) prevent scripts from
reading file input field value (really no reason for them to).

/mz

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er, I meant file inputs.

I'd have no problem with preventing focus() on file inputs, myself!

> You mean you can only select files via the file open dialog, not by typing
> paths?

Yes.

Unfortunately, an number of sites rely on reading file input contents, so we can't change that easily.

Comment 6

[Ben Bucksch (:BenB)]

> I'd have no problem with preventing focus() on file inputs, myself!

dito

> an number of sites rely on reading file input contents

uh. The path is none of their business. I may give them that file, but I may not want to disclose the path, and with it my username, local disk organization etc..

Which sites are that and what are they using it for? Detection of file type using extension, to predict whether the file will be accepted? We could either 

Comment 7

[Ben Bucksch (:BenB)]

Paul Szabo   [e-mail] wrote:

Please see also:

  bug 290478
  http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-2894
  bug 56236
  bug 258875

and further references therein.

---

Michal wrote:

This probably explains why the core of the problem wasn't fixed for
Firefox: reports were repeatedly reduced to an issue with hiding file
input fields by manipulating opacity or visibility (in my example, I
placed the box off-screen to the left, at negative absolute coords,
instead). A proper solution would be to restrict the ability for scripts
to manipulate focus and read contents of file input fields, instead.

/mz

Comment 9

[Michal Zalewski]

I don't think this is accurate. Bug 56236 is about temporarily disabling control to prevent it from receiving certain keystrokes. 

This one is about altering keyboard focus to redirect keystrokes from a regular text input box into a file input box.

To make things worse, bug 56236 is marked as a duplicate of bug 258875, which is quite unrelated (hiding file input controls by altering their opacity).

There were several other variants reported in the meantime (selectively blocking events by returning false on key event handler, etc), but they all seem different, and now they all lead to 258875, which is really unrelated?

Comment 10

[Ben Bucksch (:BenB)]

Jesse! You marked bug 56236 a dup of bug 258875, which is decisively a *different* problem, and it's still dupped. The fact that reports have been *wrongly* dupped bug 56236 and/or bug 258875 is exactly the mismanagement that has been criticized (see comment 7).

---

From: Michal Zalewski <[e-mail]>
Subject: Firefox/MSIE focus stealing vulnerability - clarification
Date: Mon, 12 Feb 2007 00:01:56 +0100 (CET)

After some research, I can offer this clarification:

  1) The MSIE 7 attack vector I described is a distinctive, new
     vulnerability that differs from the attack reported by Charles
     McAuley and Bart van Arnhem. Attacks described by them were
     fixed in MSIE7 (although MSIE6 is still exposed to the original
     flaw).

     My vulnerability attacks the same form control, but in a different
     manner. Again, the demo for this vulnerability is here:
     http://lcamtuf.coredump.cx/focusbug/ieversion.html

  2) The Firefox attack vector is related to the Charles' CVE-2006-2894,
     which in turn was a rediscovery of a problem known to Mozilla since
     2000 (!); attempts to fix it in official releases failed because the
     problem was repeatedly marked as a duplicate of a too narrowly
     defined issue with control hiding. A broader redesign probably
     eliminated the issue in development branches, but it still affects
     Firefox 1.5 and 2.0.

     This can be considered an independent rediscovery and a more
     practical demonstration of a previously reported vulnerability.
     The exploit is here: http://lcamtuf.coredump.cx/focusbug/index.html

Regards,
/mz

Comment 11

[Jesse Ruderman]

You're right, this isn't exactly a dup of bug 56236.  But the result is the
same: if you can get a visitor to type enough characters into your web page,
you can read her files.

Bug 56236 is marked as a duplicate of bug 258875 because the solution we chose
in that bug for trunk also takes care of bug 56236.  We could have marked bug
56236 as "Fixed" rather than "Duplicate of bug 258875"; either way, you'd have
to read the comments there to understand how and where it is fixed.

If we decide to take some other fix for bug 56236 on branch, we'll need to keep
in mind that disabling the control is not the only exploit mechanism, and that
focus-switching works as well.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> uh. The path is none of their business. 

I happen to agree, and that's bug 143220.

But you don't need the whole path to build up a filename; you only really need to keep track of what the last bit is.  And you can even do that without ever reading the .value -- just keep track of the key events and all.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yeah, I think we should restrict focus() calls on branch (no need on trunk though).

Either we could disable it entierly, or we could make it such that calling focus() empties the current value?

Comment 14

[Boris Zbarsky [:bzbarsky]]

I think just disabling it entirely is better.  Less surprise for users that way.

Comment 15

[Jesse Ruderman]

Would we do something about bug 56236 on branch too then?

Comment 16

[Boris Zbarsky [:bzbarsky]]

I don't think we need to, if the page can't focus the file input programmatically...

Comment 17

[Jesse Ruderman]

A malicious page that can get you to type a bunch of text would have no trouble getting you to click something (that happens to be, but doesn't look like, a file upload control) before typing.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Hmm.  Fair enough...

Comment 19

[Todd Day]

Not sure if this is the correct place to suggest this, but would a better solution in the long run be to present a warning box to the user anytime a local file is going to be uploaded to a webserver?  Firefox comes out of the box with 2 or 3 fairly inane security warnings about simple everyday things like form data...  I think a local file upload would also be a good thing to be warned about.  If this warning was already in place, the above bug (and future ones like it) would not be so much of an issue.

Comment 20

[Jordan M.]

(In reply to comment #19)
> Not sure if this is the correct place to suggest this, but would a better
> solution in the long run be to present a warning box to the user anytime a
> local file is going to be uploaded to a webserver?  Firefox comes out of the
> box with 2 or 3 fairly inane security warnings about simple everyday things
> like form data...  I think a local file upload would also be a good thing to be
> warned about.  If this warning was already in place, the above bug (and future
> ones like it) would not be so much of an issue.
> 
I'd say we catch them a step ahead of that. If we can either warn the user of the input action, or restrict actions of this input type somehow, that would probably be safer, or I could be blabbering on about the impossible/irrelevant.

Comment 21

[Mike Connor [:mconnor]]

There isn't quite enough time to get a safe and baked fix into 1.8.1.3, should look to get a fix into 1.8.1.4 as soon as possible.

Comment 22

[Johnny Stenback (:jst)]

Smaug, any chance you could look at this one? I'm not sure what's easier here, prevent focus changes while handling certain events, or force all events relating to a single key press to go to the element where the key event sequence was started.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Attachment: changing the way .focus() works with <input type="file">

this makes fileinputcontrol.focus() to focus the button, not the text 
input field. Tabbing and clicking the text input field does still work.
Would this be good enough workaround for branches?

(I tried to hack keydown, keypress and keyup events too, but that patch
started to become very ugly and hackish.)

Comment 24

[Jesse Ruderman]

I think that would fix this bug but not bug 56236, which has the same impact on security.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Would that make it impossible to use the textfield in a

   <input type="file" onfocus="this.focus()">

?  That might be ok, but worth checking...

Comment 26

[Olli Pettay [:smaug][[e-mail]]]

Actually with the patch and having capturing focus listener in an ancestor, it is possible to get "too much recursion" :(
Better patch and a testcase coming.

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v2

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Comment 29

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #24)
> I think that would fix this bug but not bug 56236, which has the same impact on
> security.
> 
Well, at least currently I'm fixing bug
titled "Focus change between onKeyDown and onKeyPress, allowing to read arbitary files using <input type=file>" ;)
Though, trying to figure out a good way to fix also bug 56236.

Comment 30

[Johnny Stenback (:jst)]

Comment on attachment 260832 [details] [diff] [review]
v2

r=jst

Comment 31

[Daniel Veditz [:dveditz]]

Comment on attachment 260832 [details] [diff] [review]
v2

approved for 1.8.0.12 and 1.8.1.4, a=dveditz

Comment 32

[Olli Pettay [:smaug][[e-mail]]]

Checked in.