405299 - Firefox file input focus stealing through label element dispatch mouse click event

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Hong]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; zh-TW; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; zh-TW; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9

Focus change allowed between onKeyDown and onKeyPress, allowing attacker to read arbitary files.

This is similar to bug 388784, but it is using label to dispatch mouse click event to change focus to textfield of file input object.

I verified this with Firefox 2.0.0.9

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Hong]

Attachment: Proof of concept

Comment 2

[:Gavin Sharp [email: [e-mail]]]

This is a duplicate of bug 404451.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Not quite.  This bug doesn't rely on a user click to focus the wrong control; it actually focuses the file by focusing the label, then puts the focus back.  I thought we'd prevented that with the label changes on branch....

The fix for bug 404451 might fix this, of course.

Comment 4

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

Taking. I have patch for this but it doesn't yet fix bug 404451.

Comment 5

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

Attachment: possible patch

I can't think of any solution which wouldn't change the behavior a bit.
The patch makes it so that clicking something in <label> has similar behavior as
calling label.focus(), meaning that input element is handled in a special way.
To keep changes in behavior as small as possible, ::SetFocus is modified to
check only type="file", not all input elements.
Although this is a small patch, I'd like to get 2 separate reviews for this. 
Just in case someone comes up with some better solution.

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 290682 [details] [diff] [review]
possible patch

Looks reasonable to me. Do we need something similar for trunk too?

r=jst, it'd probably be good if bz could look at this as well.

Comment 7

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

Comment on attachment 290682 [details] [diff] [review]
possible patch

On trunk <input type="file"> works in a different way. There typing to the textfield isn't possible.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 290682 [details] [diff] [review]
possible patch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Verified for branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.12pre) Gecko/2008011803 BonEcho/2.0.0.12pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12pre) Gecko/2008011803 BonEcho/2.0.0.12pre.

Comment 11

[Alexander Sack]

distro patches block 1.8.0.15 

Comment 12

[Alexander Sack]

Comment on attachment 290682 [details] [diff] [review]
possible patch

a=asac for 1.8.0.15

approving unmodified distro patch.

Comment 13

[Reed Loden [:reed]]

MOZILLA_1_8_0_BRANCH:

Checking in content/html/content/src/nsHTMLLabelElement.cpp;
/cvsroot/mozilla/content/html/content/src/nsHTMLLabelElement.cpp,v  <--  nsHTMLLabelElement.cpp
new revision: 1.87.6.1.2.2; previous revision: 1.87.6.1.2.1
done