Mozilla can upload files without user confirmation

RESOLVED FIXED

Status

()

Core
Security
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Met - Martin Hassman, Assigned: sicking)

Tracking

({fixed1.7})

Trunk
x86
Windows 2000
fixed1.7
Points:
---
Bug Flags:
blocking1.7 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix] vuln 1.7a/b, fixed 1.7rc1 doesnotaffect1.4)

Attachments

(3 attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7) Gecko/20040425
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7) Gecko/20040425

Description:
In the last Mozilla versions (the M1.7b and later - detail list see bellow) can
malicious JavaScript code upload files from the user computer without user
confirmation.

This is happening via classical <input type="file"> tag. The value of this tag
should be read only. If you give it some value="" in the HTML file, it should be
ignored. But unfortunately if you create input type="file" via document.write(),
its value is not cleared!!! e.g.: "document.write('<input name=\'u\' type="file"
value="c:\\autoexec.bat">');" can (after some onload="document.f.submit()")
upload "c:\autoexec.bat" to the server.

List of tested Mozilla versions (I have tested only Windows versions, sorry):
M1.6 not affected
Firefox 0.8 not affected
M1.7a not affected

M1.7b AFFECTED!
M1.7 last build AFFECTED! (Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7) Gecko/20040427)

See attachment for example (look at the value in input box).

If you want to see malicious code in action, look at the
http://www.penguin.cz/~met/pub/bug/form.html , but beware! During onload it will
automatically send your c:\autoexec.bat (sorry for Linux users 8-) to the server
and display it as the result in iframe bellow (yes, autoexec.bat is in the
Win2000/XP completely blank, so if you want to really test it, write some REM
comments in it).

(Ehm, sorry, but there is some problem on the penguin.cz server. Sometimes it
loads main www.penguin.cz page instead of my URL. So if you see blank page with
penguin logo, be patient and reload 8-)


Reproducible: Always
Steps to Reproduce:
(Reporter)

Comment 1

13 years ago
Because this is seciruy bug and is not listed by default, adding some Mozilla staff.
(Reporter)

Comment 2

13 years ago
Created attachment 147219 [details]
Tescase

Simple testcase - loog at the input tag - Mozilla does not ignore its value.
(Reporter)

Updated

13 years ago
Flags: blocking1.7?
I was sure we had a check for this... is the document.write necessary, or does
just setting the value attribute in HTML also work?

Updated

13 years ago
Flags: blocking1.7? → blocking1.7+
(Reporter)

Comment 4

13 years ago
(In reply to comment #3)
> is the document.write necessary, or does
> just setting the value attribute in HTML also work?

Yes, it is. (I am demonstrating it on the the metioned page with "malicious code
in action").

In summary:

INPUT tag in HTML - OK
INPUT tag created by DOM createElement - OK
INPUT tag created by document.write() - VULNERABLE!
I'll have a look at this
(Reporter)

Comment 6

13 years ago
Created attachment 147261 [details]
Testcase2 - innerHTML

And finaly:

INPUT tag created by innerHTML has the same problem as document.write() - see
testcace 2
This regressed between 2004-03-03-08 and 2004-03-04-08.  Bug 232706 seems very
very suspect.
Though the diff to nsHTMLInputElement looks ok....
OK, if I change that SetValue() call in nsHTMLInputElement::ParseAttribute to
SetValueInternal() (which is what it should be to start with), then this bug
goes away, as far as I can tell.  And the bug is not present if the attribute
order is reversed -- that's handled by the implementation of Reset().

So clearly the problem is that when there is script on the callstack the call to
SetValue fails (which makes sense).  The part I don't get is why it worked before!
i just arrived at the same conclusion. The reason it worked before is that we
called SetValue *before* chaning mType. What i can't figure out is why there's a
difference using document.write or using normal markup :)
> What i can't figure out is why there's a difference using document.write or
> using normal markup :)

Easy.  In one case, there's JS on the stack, in the other there is not.  Then
the security manager does its security check, decides we were called from JS, and...

This exact issue came up in discussion recently, in fact -- that our DOM apis
are not reliable for internal use because they often perform security checks and
have to guess at who the "real caller" is.
aHA! Now i figured that part out too. And this is scary stuff.

The reason that just using markup doesn't work is that then when we call
SetValue we'll actually be allowed to set the value, even though it's a
file-input!! The reason is that we don't have any javascript contexts on the
stack so UniversalFileRead is *enabled*. Which is such a scary design, but as i
recall it it's really hard to change now.

I wonder if it's somehow possible to trigger this in older versions of mozilla too.

But the right fix is to call SetValueInternal, patch comming in a second.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 147273 [details] [diff] [review]
this should do it
Attachment #147273 - Flags: superreview?(bzbarsky)
Attachment #147273 - Flags: review?(bzbarsky)

Comment 14

13 years ago
Comment on attachment 147273 [details] [diff] [review]
this should do it

The word you've entered isn't in the dictionary. Click on a spelling suggestion
below or try again using the search box to the right.

Suggestions for cought:

	 1. caught
Comment on attachment 147273 [details] [diff] [review]
this should do it

s/cought/caught/ and s/accidently/accidentally/, and r+sr=bzbarsky
Attachment #147273 - Flags: superreview?(bzbarsky)
Attachment #147273 - Flags: superreview+
Attachment #147273 - Flags: review?(bzbarsky)
Attachment #147273 - Flags: review+
Comment on attachment 147273 [details] [diff] [review]
this should do it

This patch should be very safe since all it does is to bypass some
security-checks that we absolutly don't want to get cought in.
Attachment #147273 - Flags: approval1.7?
Comment on attachment 147273 [details] [diff] [review]
this should do it

checked in on trunk
I was thinking if there was any way to trigger the same bug in the way the code
was previously written, i.e. if we need to backport this patch to the 1.0 or 1.4
branches. However afaict the old way should be safe as well.
-> sicking, his patch and check-in
Assignee: security-bugs → bugmail
Comment on attachment 147273 [details] [diff] [review]
this should do it

a=dveditz for 1.7 branch. Please add the fixed1.7 keyword when checked in
Attachment #147273 - Flags: approval1.7? → approval1.7+
checked in
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.7
Resolution: --- → FIXED
(Reporter)

Comment 22

13 years ago
Tested with "Mozilla/5.0 (Windows; U; Windows NT 5.0; cs-CZ; rv:1.7)
Gecko/20040501 (from nightly/latest-1.7/)".

Both testcases document.write() and innerHTML seem OK.
Adding Jon Granrose to CC list to help round up QA resources for verification
Whiteboard: [sg:fix] vuln 1.7a/b, fixed 1.7rc1
Whiteboard: [sg:fix] vuln 1.7a/b, fixed 1.7rc1 → [sg:fix] vuln 1.7a/b, fixed 1.7rc1 doesnotaffect1.4
Removing security-sensitive flag for bugs on the known-vulnerabilities list
Group: security

Comment 25

13 years ago
Note: The Common Vulnerabilities and Exposures project (cve.mitre.org) has
assigned the name CAN-2004-0759 to this issue.
You need to log in before you can comment on or make changes to this bug.