983326 - password fields in subframes of about: pages are treated as "insecure" (insecure password field warning appears for about:accounts)

Status: RESOLVED INCOMPLETE

(Core :: Security, defect)

Description

[Peter deHaan [:pdehaan]]

Steps to reproduce:
1. Fire up Firefox Nightly.
2. Open the developer console.
3. Navigate to `about:accounts`.


Actual results:
Console tab shows the following message:
"""
Password fields present on an insecure (http://) page. This is a security risk that allows user login credentials to be stolen. [Learn More](https://developer.mozilla.org/en-US/docs/Security/InsecurePasswords)
"""

This only seems to happen in about:accounts since it isnt really using a protocol.


Expected results:
Not that. Never that. #imscared


See also:
https://bugzilla.mozilla.org/show_bug.cgi?id=952316
https://bugzilla.mozilla.org/show_bug.cgi?id=965465

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This is a Firefox bug, actually.

The root issue is that checkIfURIisSecure in toolkit/components/passwordmgr/InsecurePasswordUtils.jsm returns false for "about:accounts", so we throw the warning from http://hg.mozilla.org/mozilla-central/annotate/b01286b4ed37/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm#l133.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

See bug 1025427 comment 1

Comment 4

[Garrett Robinson [:grobinson]]

Hm, this is a bit weird. The logic in _checkIfURIisSecure uses nsIProtocolHandler and performs the same checks as MCB to be consistent. I would expect the about: protocol to have the URI_IS_LOCAL_RESOURCE flag set. tanvi, any idea why it does not, or if it should?

We can always whitelist "about:" schemes in this function if we have to, but I'd like to make sure this isn't exposing an underlying inconsistency first.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

about: URIs can point to arbitrary resources, not all of them local (e.g. about:credits is an alias for http://www.mozilla.org/credits/).

That function should probably whitelist about: URIs for the purposes of the "secure" check, or somehow look at the post-newChannel URI.

Comment 6

[Tanvi Vyas[:tanvi]]

There are two categories of about: urls - nsAboutProtocolHandler and nsSafeAboutProtocolHandler.  about: urls fall are considered "safe" (moz-safe-about) and are included in URI_SAFE_TO_LOAD_IN_SECURE_CONTEXT.  about:accounts should perhaps be marked as safe by setting URI_SAFE_FOR_UNTRUSTED_CONTENT?  That would fix this issue.

http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsAboutProtocolHandler.cpp#208
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsAboutProtocolHandler.cpp#77
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsIAboutModule.idl#21

Comment 7

[Mark Hammond [:markh] [:mhammond]]

(In reply to Tanvi Vyas [:tanvi] from comment #6)
> about:accounts should perhaps be marked as safe by setting
> URI_SAFE_FOR_UNTRUSTED_CONTENT?  That would fix this issue.

I *think* that this flag would mean aboutaccounts.xhtml would not get chrome permissions, which it needs.

Daniel, is my understanding correct?  If it is, I don't think that is a viable option (or more specifically, would be a very difficult option to implement)

Comment 8

[:Gavin Sharp [email: [e-mail]]]

about:accounts cannot be URI_SAFE_FOR_UNTRUSTED_CONTENT because it needs chrome privileges.

Comment 9

[[e-mail]]

(In reply to Gavin Sharp from comment #5)
> about: URIs can point to arbitrary resources, not all of them local (e.g.
> about:credits is an alias for http://www.mozilla.org/credits/).

(In reply to Tanvi Vyas from comment #6)
> There are two categories of about: urls - nsAboutProtocolHandler and
> nsSafeAboutProtocolHandler.  about: urls fall are considered "safe"
> (moz-safe-about) and are included in URI_SAFE_TO_LOAD_IN_SECURE_CONTEXT. 
So this means that about:credits is considered safe to load in a frame on an https page, even though it's really an http link? (In before Gavin asks me to file a bug on changing about:credits to point to https://www.mozilla.org/credits/ .)

Comment 10

[Tanvi Vyas[:tanvi]]

(In reply to [e-mail] from comment #9)
> (In reply to Gavin Sharp from comment #5)
> > about: URIs can point to arbitrary resources, not all of them local (e.g.
> > about:credits is an alias for http://www.mozilla.org/credits/).
> 
> (In reply to Tanvi Vyas from comment #6)
> > There are two categories of about: urls - nsAboutProtocolHandler and
> > nsSafeAboutProtocolHandler.  about: urls fall are considered "safe"
> > (moz-safe-about) and are included in URI_SAFE_TO_LOAD_IN_SECURE_CONTEXT. 
> So this means that about:credits is considered safe to load in a frame on an
> https page, even though it's really an http link? (In before Gavin asks me
> to file a bug on changing about:credits to point to
> https://www.mozilla.org/credits/ .)

Oh, that's not good.  Does about:credits have the URI_SAFE_TO_LOAD_IN_SECURE_CONTEXT flag?

Looks like we may need to go through http://kb.mozillazine.org/About_protocol_links (assuming its complete) and figure out if we are correctly classifying the mixed content state of about: loads.

Comment 13

[Tanvi Vyas[:tanvi]]

(In reply to Peter deHaan [:pdehaan] from comment #0)
> Steps to reproduce:
> 1. Fire up Firefox Nightly.
> 2. Open the developer console.
> 3. Navigate to `about:accounts`.
> 
> 
> Actual results:
> Console tab shows the following message:
> """
> Password fields present on an insecure (http://) page. This is a security
> risk that allows user login credentials to be stolen. [Learn
> More](https://developer.mozilla.org/en-US/docs/Security/InsecurePasswords)
> """
> 
> This only seems to happen in about:accounts since it isnt really using a
> protocol.
> 
> 
> Expected results:
> Not that. Never that. #imscared
> 

I don't see this problem in Firefox Nightly 51.0a1 (8-29-16).  Was it fixed?

Comment 14

[Daniel Holbert [:dholbert]]

Seems fixed to me, too (in recent Nightly), FWIW.  (I had hit this before, in Firefox 33, and filed dupe bug 1025427.)

Comment 15

[Hani Yacoub, Desktop QA]

"about:accounts" link doesn't exist anymore. marking the as resolved:incomplete.