134370 - Moz displays my ftp password in 24-pt bold font

Status: VERIFIED FIXED

(Core Graveyard :: Networking: FTP, defect, P1)

Description

[Jesse Ruderman]

Builds 03/14 and 03/29.

Steps to reproduce:
1. Go to ftp://anonymous:[e-mail]/ .

Sure, the password is already on the screen, but displaying it again in an <h1>
is about as secure as yelling the password across a computer lab.

Comment 1

[Bradley Baetz (:bbaetz)]

Grr. I thought I fixed this in bug 111117. While that code does fix the 300
line, we don't actually use that. Wonder why it worked for me then...

Taking.

Comment 2

[Bradley Baetz (:bbaetz)]

Attachment: v1

OK, here we go. This patch has a side effect - if you click on a link, you will
get prompted for your password again. This is because the link on the page
doesn't have the password embedded in it, so the ftp connection cache will
consider the two hosts to be different.

This is sort of a feature, although since you can use the urlbar autocomplete
to complete any url entered this way, I can only consider usernames if people
think that that would be a better way of doing this.

Comment 3

[Jesse Ruderman]

Please don't make ftp prompt for a password each time I click a file or
directory.  That would make the passworded-ftp-site feature useless, which would
be worse than having this kind of security hole.

IMO, the best solution would be to handle FTP passwords like HTTP basic auth
passwords: keep them for the session and allow the user to save them for future
sessions using a checkbox, but don't display them on the screen.

Comment 4

[Bradley Baetz (:bbaetz)]

No, just the first time after using a url wioth your password embedded in it.

There is a bug on an ftp auth cache, yes.

Comment 5

[Jesse Ruderman]

bbaetz: ok, that's not as bad.  Does your patch depend on bug 124561? 
ftp://odin.ac.hmc.edu/foo just gives me "login incorrect" without asking for my
password.

Comment 6

[Bradley Baetz (:bbaetz)]

This bug doesn't but doing what you want does.

You can use teh ftp://user@host/ form and then you will be prompted for the
password.

Comment 7

[Darin Fisher]

Comment on attachment 76862 [details] [diff] [review]
v1

sr=darin

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 76862 [details] [diff] [review]
v1

r=dougt

Comment 9

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

I'm a little worried about the UI issue here, especially for someone who gets to
the password-protected FTP site by clicking on a link.  They may not have
noticed what the password in the URL was.  Worse yet, in a kiosk or other chrome
setup where the URL is never visible (and we do have those), they couldn't get
the password out of the URLbar.

Comment 11

[Bradley Baetz (:bbaetz)]

I thought about this a bit, and I'm not sure. If we had a an auth cache, this
would be moot, but we don't.

Is the sort of user who has a ? Or should I just embed it in the page, but not
show it? ns4 strips it out, and I think we should, too.

Comments?

Comment 12

[Jesse Ruderman]

For now, I think you should embed the password in the links but not show it in
the <h1>.  The password is already visible in the location bar, so adding it to
the status bar when a link is hovered isn't a big deal.

Comment 13

[Bradley Baetz (:bbaetz)]

Attachment: v2

OK, done.

Comment 14

[Doug Turner (:dougt)]

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

no other protocols that we have to worry about?

r=dougt

Comment 15

[Darin Fisher]

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

Index: netwerk/streamconv/converters/nsIndexedToHTML.cpp

>-    char* spec = nsCRT::strdup(baseUri.get());
>+    char* spec = nsCRT::strdup(titleUri.get());
>     nsUnescape(spec);

the strdup seems unnecessary here... how about this instead:

NS_UnescapeURL(titleUri);
const char *spec = titleUri.get();

either way, sr=darin

Comment 16

[Bradley Baetz (:bbaetz)]

NS_Escape* modifies its arguments directly, and I plan on wanting the escaped
version when I linkify the title thingy, and make a few other minor cosmetic
changes later.

Checked onto trunk, mailed drivers for approval

Comment 17

[Randell Jesup [:jesup] (needinfo me)]

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

a=[e-mail] for branch checkin

Comment 18

[Bradley Baetz (:bbaetz)]

Checked onto MOZILLA_1_0_0 branch

Comment 19

[benc]

VERIFIED: Mozilla 1.1b
the password appears in the links by way of the BASE in the HTML page.

I personally don't like the password to appear anywhere, except the URL bar (if
typed), but I think this is the best we can do w/o an auth manager for FTP.

If you bookmark one of these links, you get the password in your bookmark
automatically, but I filed bug 160471 on that. Otherwise, I can't think of any
security concerns.

The auth cache RFE is bug 80652.