97754 - HTML special characters are not commented in FTP directory listings

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[3APA3A]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.3) Gecko/20010801
BuildID:    2001080110

If filename in FTP directory contains HTML tags this tags will be part of HTML
page with file listing. Javascript in filenames will be executed. There is
security impact (for example user who can only upload files but cann't modify
uploaded files can modify directory listings) but it's not high though.

Reproducible: Always
Steps to Reproduce:
1. on ftp server create (or upload) file with HTML tag in the name (example:
 touch "/ftp/incoming/123/<img src=\"javascript:alert('Hello World')\">" or
upload file with name like this via ftp.
2. open FTP URL (example: ftp://server/incoming/123)
. 

Actual Results:  You will see "Hello world" alert.

Expected Results:  to see <img src=\"javascript:alert('Hello World')\"> as a
file name

Comment 1

[Bradley Baetz (:bbaetz)]

I can't test this at the moment, but confirming based on 
http://lxr.mozilla.org/seamonkey/source/netwerk/streamconv/converters/nsIndexedT
oHTML.cpp#164.

We need to unescape this, and then entitise it (> -> >, etc). Do we have a 
generic function to do that which is available from necko?

-> critical

Comment 2

[Bradley Baetz (:bbaetz)]

err. Note to self: do not comment on bugs after spending 9 hours on a flight. 
Lets try this again.

Theres not that much of a security impact - you can't do anything you couldn't 
do as part of a webpage, right. Its unexpected, and its wrong (and a url with a 
< in it probably stuffs up later entries), but I can't think of an exploit.

Comment 3

[benc]

Can you use it to add inline graphics to an FTP display? That would solve some
aspects of bug 90695. :)

Comment 4

[Doug Turner (:dougt)]

I can't seam to make this work.  I do see the inline script  being to be
evaulated, but nsJSThunk::EvaluateScript() prevents this because the underlying
netlib channel  (nsStreamIOChannel) does not have an interface requestor.

I do not think that this is critical.   Mitch, do you concur?  

Comment 5

[Bradley Baetz (:bbaetz)]

benc: I've done the icons in my dirviewer rewrite.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

I can't reproduce this either, but it's got me worried. If there were a way to
insert a script into an FTP directory listing, it would run with the system
principal (=the system is compromised). IMG SRC=javascript:... doesn't seem to
do anything when I tried it, and I can't seem to insert <SCRIPT> tags this way
either, because they don't work without the corresponding </SCRIPT>, and I can't
figure out how to escape the slash in the filename. There may be a way though.

HTML tags in filenames are definitely being parsed. I tried naming a file
"para<p>para" and listing the directory via ftp, and got the new paragraph in
the middle of the name. 

So there's a possiblility that this is critical. Would it be difficult to escape
HTML tags in filenames? Let's do this if we can; otherwise I think we're asking
for trouble.

Comment 7

[Doug Turner (:dougt)]

Run with the system principal?  How so?  

I do not see how this is different than a webpage.  Please advise.

Comment 8

[3APA3A]

OK, it looks like <IMG SRC="javascript:"> doesn't work each time (I was able to 
reproduce it for few times though). There is another way to execute javascript 
or anything you want through this bug, check ftp://ftp.sandy.ru/incoming/6
I've created file with name <META HTTP-EQUIV="Refresh" CONTENT="1; 
URL=malware.htm"> in the ftp://ftp.sandy.ru/incoming/6 and put mailware.htm 
into /incoming (for reasons I don't understand this url requests 
ftp://ftp.sandy.ru/incoming/malware.htm instead of 
ftp://ftp.sandy.ru/incoming/6/malware.htm, but who cares?).

BTW: is it right <META> tag is accepted in document body outside header? I 
think it is not correct, but I'm not sure about HTML standards.
BTW[2]: it looks there are 2 more bugs in FTP
 1. It's impossible to enter empty directory
 2. Directory content is cashed. If you remove or add some files to directory 
there is no way to refresh directory content without clearing the cash. 
Have I open new bids for these bugs?

Comment 9

[Bradley Baetz (:bbaetz)]

Um. How does this run with the system prinicpal? This is a stream converter - if
stream converters have any elevated privs then we run into bug 51442 again,
don't we?

Comment 10

[Bradley Baetz (:bbaetz)]

I've fixed this in my rewrite. The fix is to add a call to nsEscapeHTML before
outputting the description.

dougt: I don't have net access at home yet, and I have my other ftp patch in the
way - can you do this?

Comment 11

[Mitchell Stoltz (not reading bugmail)]

I don't know if this runs with the system principal or not, but regardless, you
shouldn't be able to sneak in HTML like this - it'll cause trouble one way or
another. Let's escape < and > as proposed. Is there a patch already posted
somewhere else?

Comment 12

[Bradley Baetz (:bbaetz)]

No patch per se - I don't have web access at home til the middle of next week.

The following untested-but-something-similar-works-in-my-heavily-hacked-tree
pseudo-diff via lxr of nsIndexedToHTML.cpp should work though:

164     nsUnescape(NS_CONST_CAST(char*, filename.get()));
+       char* htmlEscaped = nsEscapeHtml(filename.get());
-165     pushBuffer.AppendWithConversion(filename);
+       pushBuffer.AppendWithConversion(htmlEscaped);
+       nsMemory::Free(htmlEscaped);
166     pushBuffer.AppendWithConversion("</a>");

This function (used by the mime code), escapes <, >, &, and ".

+patch, review. I guess.

Comment 13

[Doug Turner (:dougt)]

reassigning to [e-mail].

Comment 14

[benc]

+makingtest

Comment 15

[Bradley Baetz (:bbaetz)]

I fixed this last week when my dirviewer patch landed. Missed marking it fixed
because this wasn't listed as a dependancy.

Comment 16

[benc]

VERIFIED: Mozilla 1.0, RC1, Win98

Comment 17

[benc]

had minor problems creating the file, I used:

touch '<img src=\"javascript:alert('Hello\ World')\">'