279562 - Copy and paste of an ftp link can reveal account/password

Status: NEW

(Core :: DOM: Serializers, defect, P5)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

1. open an remote html document using an ftp URL with account/passwd, for example
     ftp://mylogin:[e-mail]/mydoc.html  (that's a fake)

2. make sure that document contains a relative link like
     <a href="foo.html">foo</a>

3. copy the sentence containing that link and paste it in a gecko-based environment
   like Mozilla Composer
4. switch to source view
5. see the url in the anchor:
     <a href="ftp://mylogin:[e-mail]/foo.html">foo</a>

Hum, to say the least...

That is caused by |nsHTMLContentSerialize::SerializeAttributes()| calling 
|NS_MakeAbsoluteURI()| even if there is some login/passwd information in the
URL. I guess that NS_MakeAbsoluteURI() should refuse to "absolutize" the URI if
the resulting URI contains login/passwd information.

I think this is a security hole, because one could copy such a link, paste it
elsewhere and save the resulting doc without even noticing the login and
password are revealed!!! Given the fact this comes from the serializer, this bug
is valid for all products based on Gecko.

Comment 1

[[e-mail]]

Maybe it would be easier to strip off the authentication information earlier,
for instance if you browse to ftp://mylogin:[e-mail]/ the title
proudly displays the original URL complete with authentication (although the
location bar displays ftp://www.example.com/).

Comment 2

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: proposed fix

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #1)
> Maybe it would be easier to strip off the authentication information earlier,
> for instance if you browse to ftp://mylogin:[e-mail]/ the title
> proudly displays the original URL complete with authentication (although the
> location bar displays ftp://www.example.com/).

That's harder since we should not change the document's URL and the whole thing,
including the page title, is based on it...

Comment 4

[Kathleen :Brade]

I agree with Neil (comment 1).  This should be fixed at an earlier location. 
Related / Similar bugs include the window title and printouts showing the
password.  I would think that printouts would be more severe holes than this bug
because people rarely look at printouts but a paste action does give a person
the opportunity to view the link (depending which application you paste into).

Daniel--regarding your patch, I'm not sure I agree that the name / login should
be stripped.  Reality may be that a login is required to access a particular
page.  I would think that stripping the password is sufficient.  I'd also like
to see the uri made absolute but with the password removed.  Thanks!

Comment 5

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #4)
> I agree with Neil (comment 1).  This should be fixed at an earlier location. 
> Related / Similar bugs include the window title and printouts showing the
> password.  I would think that printouts would be more severe holes than this bug
> because people rarely look at printouts but a paste action does give a person
> the opportunity to view the link (depending which application you paste into).

But we can't change the document URL or we'll be unable to publish. So are
you advocating in favor of a new URI attached to DOMNSDocument, and make a few
changes so client codes use that one instead of the original URI ?

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #4)

> Daniel--regarding your patch, I'm not sure I agree that the name / login should
> be stripped.  Reality may be that a login is required to access a particular
> page.  I would think that stripping the password is sufficient.  I'd also like
> to see the uri made absolute but with the password removed.  Thanks!

Hmmmm... So the client would ask for the password ? Hmmm, nice idea.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Sounds to me like this code wants to use nsIURIFixup::CreateExposableURI.

Sounds also like the title code wants to do the same thing, as does the printout
code.

May I ask why this is filed on Firefox instead of Core, exactly?

Comment 8

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #7)
> Sounds to me like this code wants to use nsIURIFixup::CreateExposableURI.
> 
> Sounds also like the title code wants to do the same thing, as does the printout
> code.
> 
> May I ask why this is filed on Firefox instead of Core, exactly?

Because I missed Core ?-)

Comment 9

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Probably bad component, we don't have a Serializer component here...

Comment 10

[Boris Zbarsky [:bzbarsky]]

DOM to Text Conversion is serializer.

Comment 11

[Jesse Ruderman]

*** Bug 330983 has been marked as a duplicate of this bug. ***

Comment 12

[Jesse Ruderman]

Making this bug report public because of the public dup.

Comment 13

[Dave Stahr]

Here's an example URL for you gurus to tinker around with:
http://testing:[e-mail]/login

What I didn't realize at first was that the status bar shows the username and password for basic authentication as well when links are "moused-over".  This was because I had alternate text loading there on the example URL.  I've now added some standard text links if you'd like to use this URL for testing.

Comment 14

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=3 years untouched DOM/Storage bug's priority.

If you have reason to believe this is wrong, please write a comment and ni :jstutte.