Closed Bug 1357835 Opened 7 years ago Closed 7 years ago

Add a pref to disable HTTP Auth on 3rd party images

Categories

(Core :: Networking, defect)

55 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: samwcurry, Assigned: dragana)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [necko-next])

Attachments

(4 files, 1 obsolete file)

Attached image HTTP 401 Prompt
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce:

The following is the logical flow of the issue. This is going to assume that "coolwebsite.com" is a content management portal that has a forums, vendor panel, and assistance portal. The website "attacker.com" is going to be hosting the content "/malicious". The "/malicious" page will respond with the following:

HTTP/1.1 401 Unauthorized
Content-Length: *
Date: *
Content-Type: text/html
WWW-Authenticate: Basic realm="www.attacker.com"
Connection: close

Example Attack
1. The attacker, given the ability to insert an <img> tag anywhere within his profile, is able to use his malicious server as the source of the image. This results in "coolwebsite.com" querying "malicious.com" for credentials each time the image is attempted to be loaded - forcing all visitors to the attackers profile to view the prompt.
2. The attacker simply waits for users to view his profile on "coolwebsite.com". The day-to-day user of "coolwebsite.com" will accidentally view his profile in the feed hundreds of times a day. Each victim is individually prompted for credentials and expect the prompt to resonate from "coolwebsite.com".
3. Some of the victims enter credentials, some do not.

Loading the malicious content
1. GET /profile?id=attacker HTTP/1.1
2. Source is loaded through Firefox
3. GET /malicious HTTP/1.1
4. Authentication Required prompt


Actual results:

Firefox asked the user to enter credentials that would be forwarded to an arbitrary attacker-owned domain post attempting to load an image.


Expected results:

Firefox should have not displayed the "Unauthorized" prompt since the content was loaded through an <img src> tag.
Here is the full report we sent over email:

Overview
----------------------------------------------
The current version of Firefox currently assists attackers by allowing "HTTP 401 Unauthenticated" prompts to be executed after being embedded into, primarily, image source tags. This may have not been an issue five or ten years ago, but as of now there are many content management systems, forums, and panels whereas users control a lot of essential information within the panel. In the end, users should not be actively prompted to "please enter your credentials" based off an invalid image loaded onto an entirely separate domain.

History of this bug
----------------------------------------------
In 2010 this same bug was reported to Google Chrome and was fixed shortly thereafter. This bug is only currently affecting (with regards to larger browser platforms) Firefox, Internet Explorer, and Safari.

Logical Flow
----------------------------------------------
The following is the logical flow of the issue. This is going to assume that "coolwebsite.com" is a content management portal that has a forums, vendor panel, and assistance portal. The website "attacker.com" is going to be hosting the content "/malicious". The "/malicious" page will respond with the following:

HTTP/1.1 401 Unauthorized
Content-Length: *
Date: *
Content-Type: text/html
WWW-Authenticate: Basic realm="www.attacker.com"
Connection: close

Example Attack
----------------------------------------------
1. The attacker, given the ability to insert an <img> tag anywhere within his profile, is able to use his malicious server as the source of the image. This results in "coolwebsite.com" querying "malicious.com" for credentials each time the image is attempted to be loaded - forcing all visitors to the attackers profile to view the prompt.
2. The attacker simply waits for users to view his profile on "coolwebsite.com". The day-to-day user of "coolwebsite.com" will accidentally view his profile in the feed hundreds of times a day. Each victim is individually prompted for credentials and expect the prompt to resonate from "coolwebsite.com".
3. Some of the victims enter credentials, some do not.

Loading the malicious content
----------------------------------------------
1. GET /profile?id=attacker HTTP/1.1
2. Source is loaded through Firefox
3. GET /malicious HTTP/1.1
4. Authentication Required prompt

Remediation
----------------------------------------------
1. GET /profile?id=attacker HTTP/1.1
2. Source is loaded through Firefox
3. Firefox notices that the 401 response is not originating from the same domain
4. The image is simply loaded as broken instead of actively prompting the user

Who's issue is this?
----------------------------------------------
One of the first things that can be assumed after contemplating this attack scenario that it's at-fault with the "coolwebsite.com" for embedding arbitrary content. As security researchers, we respectfully disagree. Websites like Yahoo, Google, Facebook, Twitter, Spotify, eBay, USAA, Wordpress, and PayPal all use this same practice whereas users are allowed to embed arbitrary inescapable image src content. It's accepted that this practice is safe because the users are not allowed to inject malicious HTML/JS since it's restricted to the src modifier.

We've reported this issue to dozens of huge websites and they've all verified that this indeed is not an acceptable behavior. After finding this issue on more-and-more larger-and-larger platforms we decided it would be best to take it directly to Mozilla since it's far too widespread.

Please let me know if you require any additional information or would like to discuss this issue.
Cross origin errors also have the following text: "WARNING: Your password will not be sent to the web site you are currently visiting!" making this attack far less likely.

We should consider if we should follow Chrome in suppressing these errors or if this is WONTFIX.
Component: Untriaged → Networking
Product: Firefox → Core
I feel as if this should follow suite with Google Chrome because there's so many vendors who still find the behavior too invasive. You could say that "yeah, the majority of people won't enter actual credentials", but it's still super annoying to have it pop up where no-one expects it to.

Bug bounty programs are having to setup these really janky proxies and filters in JS (and they're paying four figures every couple weeks to security researchers for pointing out things innately wrong with a browser) that are very iffy and it shouldn't be like that. Either (1) Mozilla supresses the bug and safari follows, or (2) the entire industry is forced to write workarounds.

I can maybe see keeping the behavior but on the contrary where is that actually used on? I don't think I've ever seen an image authentication portal that has the user enter credentials for each image loaded.

Do you not feel as if it would be better just to fix it?
> Do you not feel as if it would be better just to fix it?

I think we should check first on how often these prompts are displayed in the first place. Firefox doesn't typically remove anything that might break websites without doing this. I would suggest as we changed the prompt to include the error I mentioned that this would have a higher breakage than we would typically want.

If you have evidence of the following there might be further rationale for the change though:
> Bug bounty programs are having to setup these really janky proxies and filters in JS (and they're paying four figures every couple weeks to security researchers for pointing out things innately wrong with a browser)

So yeah, I wasn't saying we shouldn't. We should just decide based on data.
Attached image Example Attempt To Fix
Telemetry for this is available here:

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-03-30&keys=__none__!__none__!__none__&max_channel_version=release%252F52&measure=HTTP_AUTH_DIALOG_STATS&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-03-02&table=0&trim=1&use_submission_date=0

124 million in one month isn't a even close to a small amount of breakage.

There is more evidence to suggest we should prevent HTTP 401 which to my knowledge wasn't done due to the amount of breakage that would cause:

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2016-09-19&keys=__none__!__none__!__none__&max_channel_version=nightly%252F51&measure=HTTP_AUTH_TYPE_STATS&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2016-08-01&table=0&trim=1&use_submission_date=0

The Chromium bug is:
https://bugs.chromium.org/p/chromium/issues/detail?id=174179

Providing an attachment of a proxying image isn't the proof I was talking about, proxying images is done for numerous reasons including performance and traffic to the website.

Gijs, I notice you were involved in bugs relating to this before. Would this categorise itself worthy of an intervention, the breakage seems too high even for that?
Given the potential upcoming widget changes in Bug 1319984 the prompt could require additional user interaction rather than just the warning instead of removing them.
Flags: needinfo?(gijskruitbosch+bugs)
Was this going to be addressed in a future patch already?
I can send evidence these proxies were developed in attempt to fix the 401 issue. I'm on mobile atm. I'll be on in 3 hours.
With regard to Yahoo there was a bug whereas a user could share a GIF in the comments section of an article.
They were very strict about what people could post and made sure that is was XSS-proof.

The bug that I found, simplified, was "hey: use an image that responds 401 and you can prompt 'enter password' by making it the source value". They decided the bug was worth $500. They fixed this by proxying the image. 

Here is an album of when I found the bug:
https://imgur.com/a/YIMed
* Take note that the "URL" parameter of the request is the raw url, and then on the next page the image of the execution.

Right after they fixed the page, this is now the image that is displayed:
https://s.yimg.com/lo/api/res/1.2/nEDXa9OY5fhi7MkGJJbmgQ--/Zmk9ZmlsbDthcHBpZD1pZXh0cmFjdA--/http://897theriver.com/admin
* https://imgur.com/a/IgkPB

The purpose of the proxy was to fix the 401 attack.

Here is an additional page that fixed the 'bug'.
https://jira.atlassian.com/browse/CONFSERVER-28932

There's many circumstances where pages either (1) white-list images [limiting user choice] or (2) proxy images.
These addressees do not fix the underlying problem, but simply the surface level one.

Twitter has currently triaged some of my reports on 401 attacks as "impactful". These companies do care about user-controlled 401 attacks.
(In reply to Jonathan Kingston [:jkt] from comment #7)
> Telemetry for this is available here:
> 
> https://telemetry.mozilla.org/new-pipeline/dist.html#!
> cumulative=0&end_date=2017-03-30&keys=__none__!__none__!
> __none__&max_channel_version=release%252F52&measure=HTTP_AUTH_DIALOG_STATS&mi
> n_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=sub
> missions&start_date=2017-03-02&table=0&trim=1&use_submission_date=0
> 
> 124 million in one month isn't a even close to a small amount of breakage.
> 
> There is more evidence to suggest we should prevent HTTP 401 which to my
> knowledge wasn't done due to the amount of breakage that would cause:
> 
> https://telemetry.mozilla.org/new-pipeline/dist.html#!
> cumulative=0&end_date=2016-09-19&keys=__none__!__none__!
> __none__&max_channel_version=nightly%252F51&measure=HTTP_AUTH_TYPE_STATS&min_
> channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submi
> ssions&start_date=2016-08-01&table=0&trim=1&use_submission_date=0
> 
> The Chromium bug is:
> https://bugs.chromium.org/p/chromium/issues/detail?id=174179
> 
> Providing an attachment of a proxying image isn't the proof I was talking
> about, proxying images is done for numerous reasons including performance
> and traffic to the website.
> 
> Gijs, I notice you were involved in bugs relating to this before. Would this
> categorise itself worthy of an intervention, the breakage seems too high
> even for that?

Honza is in a better position to answer this. I don't really know - Chromium seems to have done it 4 years ago, but only for images.

It's not possible from the telemetry to see how many of the subresources are images, or how many of those cases are legitimate. I also expect that it's under-counting, because it's only opt-in users, and we don't know how many of those cases were legitimate, and we don't know how "concentrated" these cases were (in the sense of, is it just a few users that see this prompt 100s of times a month, or is it loads of users that see this occasionally, or...). The other weird thing is that a huge number of the cross-domain subresource requests are coming from Windows 7. On all other OSes (including other versions of Windows), XHRs are by far the most common. Which makes me suspect something is hitting this in some form of automation or whatever. Best check with telemetry folks before drawing too many conclusions.


> Given the potential upcoming widget changes in Bug 1319984 the prompt could
> require additional user interaction rather than just the warning instead of
> removing them.

There's no assignee for that bug, no design, and the last discussion was several months ago. At the moment it doesn't seem likely that there will be a fix there soon. I also worry that we will see that bug as some kind of panacea, which it is unlikely to be.


FWIW, I believe that in the past we've considered the fact that the prompt includes the domain requesting credentials (ie the x-domain origin, not the page's origin) as sufficient protection for users here. I don't know if it's time to change that stance. Dan?
Flags: needinfo?(honzab.moz)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dveditz)
My stance on the entire thing is simply this: there shouldn't be the capability whereas someone in control of "img src" can make a dialogue that sends credentials. The function of "img src", defined by RFC, is to display an image. It seems overly permissive that someone can generate dialogue/prompts to remote hosts through something so vanilla.
Whiteboard: [necko-next]
This is a dupe of bug 647010, although more limited if it's only asking this for images. The stats jkt cites in comment 7 were added in bug 979359 to see if we could make this change, and in fact we DID in Firefox 40. Then all hell broke loose (bug 64701 comment 61 and following, and other bugs). We backed out the change in bug 1197944, shipped a hot-fix to Firefox 40 users (bug 1201065), and instead added the cross-site warning to the dialog (that you find insufficient) in bug 1230462.

It would be nice if telemetry broke this down by request type more, and not just XHR vs everything else. We'd have to measure the occurrence of images specifically if we wanted to consider this request.

> History of this bug
> ----------------------------------------------
> In 2010 this same bug was reported to Google Chrome and was fixed shortly thereafter.

That's earlier than I see. They announced mid-2011 that they'd be doing it later that year https://blog.chromium.org/2011/06/new-chromium-security-features-june.html. Then they ran into the problems we did and backed it out, and later in mid 2013 settled on the "only block images" approach https://bugs.chromium.org/p/chromium/issues/detail?id=174179

That's at best a partial fix, but maybe there are enough sites that allow embedding images but not anything else that it would be helpful.

Switching needinfo from Honza to Dragana since she worked on the earlier bugs (sorry to drag you back in).
Flags: needinfo?(honzab.moz)
Flags: needinfo?(dveditz)
Flags: needinfo?(dd.mozilla)
I will add telemetry and I will extend pref to disable images if we decide to do it.
Assignee: nobody → dd.mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(dd.mozilla)
Attached patch bug_1357835_v1.patch (obsolete) — Splinter Review
Attachment #8863257 - Flags: review?(tanvi)
Attachment #8863257 - Flags: review?(honzab.moz)
Attachment #8863257 - Flags: review?(francois)
Comment on attachment 8863257 [details] [diff] [review]
bug_1357835_v1.patch

Review of attachment 8863257 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/Histograms.json
@@ +2056,5 @@
>        "high": 60000,
>        "n_buckets": 100,
>        "description": "Time in milliseconds that http channel spent suspended between AsyncOpen and OnStartRequest."
>    },
> +  "HTTP_AUTH_DIALOG_STATS_V2": {

nit: the usual convention would be `HTTP_AUTH_DIALOG_STATS_2`

Not worth changing unless you're making other changes though.

::: toolkit/components/telemetry/histogram-whitelists.json
@@ +285,5 @@
>      "HISTORY_LASTVISITED_TREE_QUERY_TIME_MS",
>      "HTTPCONNMGR_TOTAL_SPECULATIVE_CONN",
>      "HTTPCONNMGR_UNUSED_SPECULATIVE_CONN",
>      "HTTPCONNMGR_USED_SPECULATIVE_CONN",
> +    "HTTP_AUTH_DIALOG_STATS_V2",

Could you add an alert email to Histograms.json instead so that we don't have to keep this probe whitelisted?

@@ +1012,5 @@
>      "HISTORY_LASTVISITED_TREE_QUERY_TIME_MS",
>      "HTTPCONNMGR_TOTAL_SPECULATIVE_CONN",
>      "HTTPCONNMGR_UNUSED_SPECULATIVE_CONN",
>      "HTTPCONNMGR_USED_SPECULATIVE_CONN",
> +    "HTTP_AUTH_DIALOG_STATS_V2",

Could you add the bug number to Histograms.json instead so that we don't have to keep this probe whitelisted?
Attachment #8863257 - Flags: review?(francois)
(In reply to François Marier [:francois] from comment #16)
> Comment on attachment 8863257 [details] [diff] [review]
> bug_1357835_v1.patch
> 
> Review of attachment 8863257 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/Histograms.json
> @@ +2056,5 @@
> >        "high": 60000,
> >        "n_buckets": 100,
> >        "description": "Time in milliseconds that http channel spent suspended between AsyncOpen and OnStartRequest."
> >    },
> > +  "HTTP_AUTH_DIALOG_STATS_V2": {
> 
> nit: the usual convention would be `HTTP_AUTH_DIALOG_STATS_2`
> 
> Not worth changing unless you're making other changes though.
> 
> ::: toolkit/components/telemetry/histogram-whitelists.json
> @@ +285,5 @@
> >      "HISTORY_LASTVISITED_TREE_QUERY_TIME_MS",
> >      "HTTPCONNMGR_TOTAL_SPECULATIVE_CONN",
> >      "HTTPCONNMGR_UNUSED_SPECULATIVE_CONN",
> >      "HTTPCONNMGR_USED_SPECULATIVE_CONN",
> > +    "HTTP_AUTH_DIALOG_STATS_V2",
> 
> Could you add an alert email to Histograms.json instead so that we don't
> have to keep this probe whitelisted?
> 
> @@ +1012,5 @@
> >      "HISTORY_LASTVISITED_TREE_QUERY_TIME_MS",
> >      "HTTPCONNMGR_TOTAL_SPECULATIVE_CONN",
> >      "HTTPCONNMGR_UNUSED_SPECULATIVE_CONN",
> >      "HTTPCONNMGR_USED_SPECULATIVE_CONN",
> > +    "HTTP_AUTH_DIALOG_STATS_V2",
> 
> Could you add the bug number to Histograms.json instead so that we don't
> have to keep this probe whitelisted?

Thanks, I will fix  this.
Attachment #8863257 - Attachment is obsolete: true
Attachment #8863257 - Flags: review?(tanvi)
Attachment #8863257 - Flags: review?(honzab.moz)
Attachment #8863263 - Flags: review?(tanvi)
Attachment #8863263 - Flags: review?(honzab.moz)
Attachment #8863263 - Flags: review?(francois)
Comment on attachment 8863263 [details] [diff] [review]
bug_1357835_v1.patch

Review of attachment 8863263 [details] [diff] [review]:
-----------------------------------------------------------------

datareview+
Attachment #8863263 - Flags: review?(francois) → review+
Comment on attachment 8863263 [details] [diff] [review]
bug_1357835_v1.patch

Review of attachment 8863263 [details] [diff] [review]:
-----------------------------------------------------------------

Tanvi asked me to do the r? for her. I think that looks good to me.

::: netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
@@ +1045,5 @@
>      case SUBRESOURCE_AUTH_DIALOG_ALLOW_ALL:
>          // Allow the http-authentication dialog.
> +        if (!sImgCrossOriginAuthAllowPref &&
> +            ((loadInfo->GetExternalContentPolicyType() == nsIContentPolicy::TYPE_IMAGE) ||
> +             (loadInfo->GetExternalContentPolicyType() == nsIContentPolicy::TYPE_IMAGESET))) {

careful: loadInfo might be null, please add |&& loadinfo && | before accessing the content policy type.
Attachment #8863263 - Flags: review?(tanvi) → review+
Would this possibly be eligible for CVE post-fix with the names "Samuel Curry" and "Jon Bottarini" included? If not it's completely fine. I just see this as a potential security risk that has been addressed awesomely by Mozilla.
Blocks: 1361848
Comment on attachment 8863263 [details] [diff] [review]
bug_1357835_v1.patch

Review of attachment 8863263 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
@@ +1042,5 @@
>          // Open the http-authentication credentials dialog for
>          // the sub-resources only if they are not cross-origin.
>          return !topDoc && !xhr && mCrossOrigin;
>      case SUBRESOURCE_AUTH_DIALOG_ALLOW_ALL:
>          // Allow the http-authentication dialog.

maybe update the comment?

::: toolkit/components/telemetry/Histograms.json
@@ +2061,5 @@
> +    "expires_in_version": "61",
> +    "alert_emails": ["necko@mozilla.com"],
> +    "bug_numbers": [1357835],
> +    "kind": "enumerated",
> +    "n_values": 32,

there are more than 40 types to report, maybe have 64 here to be safe?

@@ +2062,5 @@
> +    "alert_emails": ["necko@mozilla.com"],
> +    "bug_numbers": [1357835],
> +    "kind": "enumerated",
> +    "n_values": 32,
> +    "description": "Stats about what kind of resource requested http authentication. (0=top-level doc, 1=same origin subresources, 2=same origin xhr 3+(nsIContentPolicy type)=cross-origin subresources per nsIContentPolicy type)"

thinking of:
equals to nsIContentPolicy's nsContentPolicyType (i.e. no offset)
61 = top-level doc
62 = same origin sub
63 = same origin xhr

so no one has to calculate and make mistakes that could mislead our decisions ;)
Attachment #8863263 - Flags: review?(honzab.moz) → review+
(In reply to Honza Bambas (:mayhemer) from comment #22)
> Comment on attachment 8863263 [details] [diff] [review]
> bug_1357835_v1.patch
> 
> Review of attachment 8863263 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/Histograms.json
> @@ +2061,5 @@
> > +    "expires_in_version": "61",
> > +    "alert_emails": ["necko@mozilla.com"],
> > +    "bug_numbers": [1357835],
> > +    "kind": "enumerated",
> > +    "n_values": 32,
> 
> there are more than 40 types to report, maybe have 64 here to be safe?
> 

There are only 23 external types the rest are internal that map to external and I am querying for external type:
loadInfo->GetExternalContentPolicyType(). Therefore 32 is enough.


> @@ +2062,5 @@
> > +    "alert_emails": ["necko@mozilla.com"],
> > +    "bug_numbers": [1357835],
> > +    "kind": "enumerated",
> > +    "n_values": 32,
> > +    "description": "Stats about what kind of resource requested http authentication. (0=top-level doc, 1=same origin subresources, 2=same origin xhr 3+(nsIContentPolicy type)=cross-origin subresources per nsIContentPolicy type)"
> 
> thinking of:
> equals to nsIContentPolicy's nsContentPolicyType (i.e. no offset)
> 61 = top-level doc
> 62 = same origin sub
> 63 = same origin xhr
> 
> so no one has to calculate and make mistakes that could mislead our
> decisions ;)

ok, I will change that.
Pushed by dd.mozilla@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a47b645bacdc
Extend telemetry for http autentication dialog prompt to show the subresource cross-origin auth dialog prompt per suresource type. Also add a pre to putentially disable auth propts for the cross-origin images. r=mayhemer r=ckerschb r=francois
https://hg.mozilla.org/mozilla-central/rev/a47b645bacdc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Hi Samuel Curry!

Your bug is just the same as my https://bugzilla.mozilla.org/show_bug.cgi?id=647010, which I informed Mozilla about in March 2011. People like to add new entries with it to Bugzilla. This is vulnerability in all browsers, which support Basic/Digest Authentication, as I wrote in my entry. So a lot of web browsers are vulnerable, not only Firefox.

I called this attack as Onsite phishing (or Inline phishing). It can be used for stealing of logins and passwords of web sites' users.
Summary: Firefox 52.0+ does not properly handle "HTTP 401" responses enabling attackers surface for malicious activity → Collect better telemetry on sub-requests that prompt for http auth
Missed the main point, hopefully this summary is a better description of what this bug turned into.
Summary: Collect better telemetry on sub-requests that prompt for http auth → Add a pref to disable HTTP Auth on 3rd party images
Blocks: 1435085
See Also: → 1692268
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: