The default bug view has changed. See this FAQ.

can drag text into file upload control

VERIFIED FIXED

Status

()

Core
Security
--
major
VERIFIED FIXED
14 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: John Keiser (jkeiser))

Tracking

({csectype-spoof, fixed1.4.3, sec-high})

Trunk
x86
All
csectype-spoof, fixed1.4.3, sec-high
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

1.46 KB, text/html
Details
6.73 KB, patch
Christopher Aillon (sabbatical, not receiving bugmail)
: review+
Brian Ryner (not reading)
: superreview+
Christopher Aillon (sabbatical, not receiving bugmail)
: approval1.4.3+
chris hofmann
: approval1.7a+
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
Mozilla allows dragging text into file upload controls.  See the testcase for
how this can be abused.

Suggested fix: allow dragging text into normal text fields but not into file
upload controls.  (This is what IE for Windows does.)

See also:
bug 50660 Should be able to drag FILES to the file upload control
bug 57770 Using styles, clipboard to confuse text entry into file upload control
(Reporter)

Comment 1

14 years ago
Created attachment 124058 [details]
Demo: "drag the smiley into the box"
(Reporter)

Updated

14 years ago
Flags: blocking1.4?
Probably internet exploder does not allow dragging into upload because IIRC
there was an exploit with only clicking which "emulated" dragging into file upload.
(Reporter)

Comment 3

14 years ago
Even if Mozilla doesn't have a bug that can turn a click into a drag, I think
this should be fixed.
Over to jkeiser. I'd realy like to see this fix get into 1.4.
Assignee: mstoltz → jkeiser
Keywords: nsbeta1
I suggest a long term solution to file upload attacks - a warning that a file is
being uploaded (probably also a warning not to turn off the first warning).
This will stop also unknown file upload attacks.
Uploading files does not seem often used, so probably it won't be very annoying.
(Assignee)

Comment 6

14 years ago
This seems reasonable.  One can drag into the browse box anyway, and that can't
be obscured.
Status: NEW → ASSIGNED
John, we think this is dangerous. Can we consider disallowing the dragging?

Comment 8

14 years ago
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-

Comment 9

14 years ago
mozilla1.4 shipped. unsetting blocking1.4 request.
Flags: blocking1.4?

Comment 10

14 years ago
john, still have cycles to look at this?

Comment 11

14 years ago
adding jst to cc list
Created attachment 137698 [details] [diff] [review]
Don't allow dropping into file inputs.

This fixes this bug, but I'm not sure I got this in the right place. I made the
plain text data transfer code prevent dropping into form controls that don't
allow it, but is that enough? It is for this case, but can others think of
other cases that this might not cover?

Updated

14 years ago
Attachment #137698 - Flags: superreview?(hjtoi-bugzilla)
Attachment #137698 - Flags: review?(caillon)
(Reporter)

Updated

13 years ago
Flags: blocking1.7a?
Comment on attachment 137698 [details] [diff] [review]
Don't allow dropping into file inputs.

I'm wondering if we can avoid getting this far altogether (when we get a drop
event in a form control which doesn't allow dropping, I think we should be able
to avoid even calling the editor methods).    If you want to land this, though,
r=caillon since this route is also fine.  One other thing I'd ask is whether we
want to also consider adding a similar check to nsHTMLDataTransfer.cpp in case
someone wants to disable dropping in their implementation for HTML widgets?
Attachment #137698 - Flags: review?(caillon) → review+
Comment on attachment 137698 [details] [diff] [review]
Don't allow dropping into file inputs.

I'm kinda leaning towards leaving the code where I originally put it, though
I'm not opposed to finding another place for it, but for now I think I'd prefer
to leave it. And I'm having a hard time imagining a situation where we'd have a
HTML widget form control that doesn't allow dropping into. So I'm going to
leave this as is unless I hear convinsing arguments for moving this check.
Attachment #137698 - Flags: superreview?(hjtoi-bugzilla) → superreview?(bryner)
Attachment #137698 - Flags: superreview?(bryner) → superreview+
Comment on attachment 137698 [details] [diff] [review]
Don't allow dropping into file inputs.

Duh, forgot to land this last night. It's a security problem, so I'd vote for
taking this for 1.7a.
Attachment #137698 - Flags: approval1.7a?

Comment 16

13 years ago
Comment on attachment 137698 [details] [diff] [review]
Don't allow dropping into file inputs.

a=chofmann for 1.7a
Attachment #137698 - Flags: approval1.7a? → approval1.7a+
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Flags: blocking1.7a?
verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b)
Gecko/20040421
Status: RESOLVED → VERIFIED
Adding Jon Granrose to CC list to help round up QA resources for verification
Comment on attachment 137698 [details] [diff] [review]
Don't allow dropping into file inputs.

a=blizzard for 1.4.3
Attachment #137698 - Flags: approval1.4.3+
Fixed on the 1.4 branch
Keywords: fixed1.4.3
OS: Windows XP → All
Whiteboard: [sg:fix]
Removing security-sensitive flag for bugs on the known-vulnerabilities list
Group: security
(Reporter)

Updated

4 years ago
Keywords: csec-spoof, sec-high
Whiteboard: [sg:fix]
You need to log in before you can comment on or make changes to this bug.