1321719 - (CVE-2017-5415) Address bar spoof using blob URI scheme

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Abdulrahman Alqabandi]

Attachment: PoC.html

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36

Steps to reproduce:

Tested on latest nightly with and without e10s. Does not work on latest stable, posting regression range soon.

Run attached PoC


Actual results:

We can change location.pathname which leads to spoof


Expected results:

It's blocked in stable

Comment 1

[Abdulrahman Alqabandi]

Attachment: mozreg-thing.png

Seems like Bug 1282407 caused the regression. 

PoC works in build 2016-07-18
PoC does not work in 2016-07-17

First build where the PoC works:

-----------------------------------------
app_name: firefox
build_date: 2016-07-17 15:25:00.705000
build_file: C:\Users\Abdulrahman\.mozilla\mozregression\persist\a1b20019c22d--mozilla-inbound--firefox-50.0a1.en-US.win64.zip
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/WlmB8WMqRySONhVW8ikCzA/runs/0/artifacts/public%2Fbuild%2Ffirefox-50.0a1.en-US.win64.zip
changeset: a1b20019c22d1a1f177387c754ec2eed74aced1b
pushlog_url: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d8201a97184f2f6cdcd70f19d8d6fc90e5589f33&tochange=a1b20019c22d1a1f177387c754ec2eed74aced1b
repo_name: mozilla-inbound
repo_url: https://hg.mozilla.org/integration/mozilla-inbound
task_id: WlmB8WMqRySONhVW8ikCzA
-----------------------------------------
Posting screenshot of mozregression in case i missed something.

Comment 2

[Abdulrahman Alqabandi]

For some reason if we do this step by step (in certain circumstance) this doesn't work. To be clear, the PoC in Comment 0 works 100% of the time. But if you follow these steps (which are pretty much the same as the original PoC) it seems like it doesn't work. Very odd...

1. Run the following:
-----------------------
<script>
location=URL.createObjectURL(new Blob(['<b>a</b>'], {type: 'text/html'}))
</script>
-------------
2. Once within blob URL, open up web console and enter the following:

location.pathname="https://www.facebook.com/login"

3. "NS_ERROR_DOM_BAD_URI: Access to restricted URI denied" error is thrown..


This is confusing since I am doing the same thing in the original PoC, right?

Comment 3

[Abdulrahman Alqabandi]

It seems like if we take longer to change pathname, it gets blocked.

---------------------------------PoC that works (it will loop)-------------------------------
<script>
location=URL.createObjectURL(new Blob(['a<script>location.pathname="https://www.facebook.com/login"<\/script>'], {type: 'text/html'}))
</script>


--------------------------------Similar PoC, except it takes 3 seconds to execute main bug (Does not work)----------------------
<script>
location=URL.createObjectURL(new Blob(['a<script>setTimeout(function(){location.pathname="https://www.facebook.com/login"},3111)<\/script>'], {type: 'text/html'}))
</script>

Comment 4

[:Gijs (he/him)]

You wrote that it doesn't work on latest stable, but bug 1282407 was fixed for 50, which is current release. Can you clarify which builds are affected in your testing?

Also, ugh, clearly we should have made time for bug 1248936 before now. :-\

baku, do you have cycles to look at this?

Comment 5

[Abdulrahman Alqabandi]

> Can you clarify which builds are affected in your testing?

Oh I was testing on an old stable version which I assumed was up to date. After updating it seems like the PoC works on actual latest stable. My apologies.

Comment 6

[Andrea Marchesini [:baku]]

Attachment: bloburl.patch

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8816920 [details] [diff] [review]
bloburl.patch

This looks like a hack and I'm still missing why this would be even correct.
If my reading of URL spec is right, blob uris should get 'cannot-be-a-base-URL flag' set meaning that setting pathname would become no-up.

Comment 8

[Andrea Marchesini [:baku]]

Attachment: bloburl.patch

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8817260 [details] [diff] [review]
bloburl.patch

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

Looks good. Thanks!
We should probably also add some tests for this at some point.

Comment 10

[Andrea Marchesini [:baku]]

Test attached to the patch before landing it.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cb095e4247e2

Comment 12

[:Gijs (he/him)]

I guess we want this uplifted, given 50 is affected?

Comment 13

[Al Billings [:abillings - ex-MoCo]]

To 51 and 52. Too late for 50 without a rebuild.

Comment 14

[Al Billings [:abillings - ex-MoCo]]

Also, why was this checked in without sec-approval? being set on the patch and then the template filled out? This doesn't even have a security rating on it (which is a "no no" for checking in security issues).

Comment 15

[Andrea Marchesini [:baku]]

First of all, I'm sorry for this. I think this bug is not a sec-high bug: it's a spoofing attach, but the awesome bar clearly shows that the domain is not the right one.

Comment 16

[Daniel Veditz [:dveditz]]

guessing maybe sec-moderate? To me this is a completely unconvincing spoof and I'd lean toward sec-low, but it might be more believable to the general population who have less experience. Won't have a lock icon, though.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Baku, it seems that we want to uplift it, could you fill the uplift request to aurora & beta? Thanks

Comment 18

[Andrea Marchesini [:baku]]

Comment on attachment 8817260 [details] [diff] [review]
bloburl.patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1282407
[User impact if declined]: the path name of a blobURL can be changed. and if this is used in location, it can confuse the user.
[Is this code covered by automated tests?]: no 
[Has the fix been verified in Nightly?]: yes - or it should be.
[Needs manual test from QE? If yes, steps to reproduce]: there is test case included.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: none
[Why is the change risky/not risky?]: We just use the correct SetFilePath() instead SetPath() method in nsIURI.
[String changes made/needed]: none

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8817260 [details] [diff] [review]
bloburl.patch

OK to uplift to 52 aurora. 
baku, do you need to rebase for beta uplift?

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e3023975fdec

Comment 21

[Carsten Book [:Tomcat]]

backed out for bustage in https://hg.mozilla.org/releases/mozilla-aurora/rev/80e89a0b39c5c21de97eb9f1ad1c60fcdc618992 

bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=4576041&repo=mozilla-aurora

Comment 22

[Andrea Marchesini [:baku]]

Bug 1310483 must be uplift as well otherwise nsIURIWithQuery is not available.

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

Wontfix for beta 51, we are in the last week of the cycle and as a sec-moderate issue I am ok leaving it to be fixed in 52.   Baku if you disagree, please let me know.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b0e83800dc6

Comment 25

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue using the poc.html from comment#0 using the following build:
* fx53.0a1, buildid: 20161203030204, changeset: 557548714db5
** https://archive.mozilla.org/pub/firefox/nightly/2016/12/2016-12-03-03-02-04-mozilla-central/

Under the awesome bar, you'll notice "blob:https://www.facebook.com/login".

Went through verification using the following builds on each platform:

* fx54.0a1, buildid: 20170301030202, changeset: 34c6c2f302e7
** https://archive.mozilla.org/pub/firefox/nightly/2017/03/2017-03-01-03-02-02-mozilla-central/

* fx53.0a2, buildid: 20170301004003, changeset: 65c9d527fc8c
** https://archive.mozilla.org/pub/firefox/nightly/2017/03/2017-03-01-00-40-03-mozilla-aurora/

* fx52.0, buildid: 20170227080736, changeset: 2183f7cb4f88
** https://archive.mozilla.org/pub/firefox/candidates/52.0-candidates/build1/

Platforms Used:

* macOS 10.12.3 x64 - PASSED
* Win 10 Pro x64 - PASSED
* Ubuntu 16.04.2 LTS x64 - PASSED

Test Cases:

* ensured it's not reproducable under e10s tabs
* ensured it's not reproducable under non-e10s tabs
* ensured it's not reproducable under private tabs
* ensured it's not reproducable under container tabs