111117 - Username and password appear in "Index of..." language

Status: VERIFIED FIXED

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

Description

[D. Brodale]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.6) Gecko/20011120
BuildID:    2001112009

Not certain whether this is a bug or whether
the behavior observed represents a preference
on the part of the developers, but...

Should a URI include a username and password -

ex. ftp://user:[e-mail]/

- the entire URI is duplicated in both the title
bar of the browser window and (in the case of ftp
sites) within the introductory "Index of..." info
for the "page" viewed.

Using the basic controls ("Up to higher level
directory", activating a directory link, etc.)
will continue to display the original user:pass
used as a user traverses an ftp site.

Although the user:pass data *is* part of the
URI - and significant, as differing users may
have differing ftp trees - I wonder whether it
is problematic to actually display the user:pass
data visibly to the user:

(1) Potentially exposes user:pass combinations to
people looking at another's screen (we'll ignore
security implications of placing user:pass in the
URL for the moement, ok?)

(2) Leads to lengthy "Index of..." strings that,
because of the nature of URIs, pushes the perhaps
more relevant information (the full pathname) off
to the right, possibly out of view for smaller
browser windows, etc.

Earlier versions of Netscape and all versions of
Internet Explorer remove user:pass when displaying
FTP URIs. Opera munges the URI by replacing only the
password with a series of six asterisks and/or dropping
it altogether and displaying only the username
used to access a site within the URI.

Such behavior also appears to occur with http and
https transports for Mozilla, though in these cases,
the user:pass appears only in the URI field, as a
document's TITLE will override presentation in the
titlebar of the window (unless it's not present?).

At best, the full presentation of URIs containing
user:pass data by Mozilla represents variance from
"what other browsers do."

Reproducible: Always

Steps to Reproduce:
1. Enter any ftp URI containing user:pass information
of the form - ftp://user:[e-mail]/

Actual Results:
Full display of URI in window titlebar and
"Index of..." listing, including user:pass
information.

Expected Results:
User:pass information should likely be obscured
or removed prior to display of URI - perhaps as
an optional feature.

Comment 1

[Bradley Baetz (:bbaetz)]

Hmm. The problem is that the initial / is relative to your login dir. I can
remove the password, I guess.

Its part of the url bar, though, so its not really secure.

We don't display it if you just enter your username, and let the browser prompt
you for your password.

Changing the entered uri from the urlbar isn't going to happen, I think. If you
want that, file a separate bug on the urlbar. They may WONTFIX it, though.

Comment 2

[D. Brodale]

Just to clarify: "url bar" is the area next
to the Back/Forward/Reload/Stop widgets? Or
is it the same thing as the titlebar on the
entire browser window?

Removing the password (and the separating
color) may be enough -- the user account
is what determines what one's "intial /"
is, and a malformed password should trigger
an error message ... so that makes sense.

Just wondering whether your use of "url bar"
== my use of "titlebar". :)

Comment 3

[Bradley Baetz (:bbaetz)]

The urlbar is the bit next to the stop button.

Comment 4

[Bradley Baetz (:bbaetz)]

Attachment: patch

OK, here we go. I've cleaned up the code, and removed some cruft while I was at
it, too.

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 60428 [details] [diff] [review]
patch

assuming that changing from nsCAutoString to nsCString is the right thing to
do.  Is it?

Comment 6

[Bradley Baetz (:bbaetz)]

Yes: Its larger than 64 bytes, and changing the param means that I can pass
arround a modern string instead.

Comment 7

[Darin Fisher]

Comment on attachment 60428 [details] [diff] [review]
patch


>+nsresult
>+nsFTPDirListingConv::GetHeaders(nsAWritableCString& headers,
>+                                nsIURI* uri)
>+{
>+    // build up 300 line
>+    headers.Append("300: ");
>+
>+    nsXPIDLCString spec;
>+    nsresult rv = uri->GetSpec(getter_Copies(spec));
>+    if (NS_FAILED(rv)) return rv;
>+
>+    // Bug 111117 - don't print the password
>+    nsXPIDLCString pw;
>+    uri->GetPassword(getter_Copies(pw));
>+    if (!pw.IsEmpty()) {
>+        // The : for the password is the second in the string
>+        PRInt32 colPos = spec.FindChar(':');
>+        colPos = spec.FindChar(':', colPos+1);
>+        NS_ASSERTION(colPos>0, "Password but no ':'?");
>+        PRInt32 atPos = spec.FindChar('@', colPos+1);
>+        NS_ASSERTION(atPos>0, "Password but no '@'?");
>+        headers.Append(Substring(spec, 0, colPos) +
>+                       Substring(spec, atPos, spec.Length()));
>+    } else {
>+        headers.Append(spec);
>+    }
>+    headers.Append(char(nsCRT::LF));
>+    // END 300:
>+
>+    // build up the column heading; 200:
>+    headers.Append("200: filename content-length last-modified file-type\n");
>+    // END 200:
>+}

how often is GetHeaders called?  if not so often, then how about this instead:

nsXPIDLCString pw, spec;
uri->GetPassword(getter_Copies(pw));
if (!pw.IsEmpty()) {
    uri->SetPassword(nsnull);
    uri->GetSpec(getter_Copies(spec));
    headers.Append(spec);
    uri->SetPassword(pw);
} else {
    uri->GetSpec(getter_Copies(spec));
    headers.Append(spec);
}

this way your code doesn't have to know how to parse an URL.  it's up to you...
if you think the added efficiency of your solution is worth the added
complexity,
then that's fine with me.

sr=darin, either way.

Comment 8

[Bradley Baetz (:bbaetz)]

Checked in with darin's cleaner suggestion.

Comment 9

[benc]

+testcase,
still seeing this in Mozilla 1.0 RC w/ our composer test account.

Will look more, reporter, what do you see?

Comment 10

[D. Brodale]

With a recent nightly (build 2002060308) and testing
ftp://anonymous:[e-mail]/
I see that the password is stripped from the "Index of..."
heading within the document display and the window's title
bar, but that both user and password are retained within
the "url bar".

This fits with the earlier suggestion that a fix for
the "url bar" would not be covered, requiring a separate
bug report, and that password (but not user) would be
stripped from the "Index of..." heading.

I don't know whether anyone wants to file a separate issue
for the "url bar", though :p

Comment 11

[benc]

Until we have an auth cache, the password has to be in the URL, otherwise you
will get reprompted.

This does create problems for things like refer lines and saving bookmarks...

Comment 12

[benc]

Filed bug 160471 on bookmarks, and checked HTTP referer lines (no problem there).

VERIFIED: Mozilla 1.1b, Win98, Mac OS X; Chimera 0.4.
I'll be checking Linux later today, but I anticipate this is fine.