Bug 1321719 (CVE-2017-5415)

Address bar spoof using blob URI scheme

VERIFIED FIXED in Firefox 52

Status

()

VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: qab, Assigned: baku)

Tracking

({csectype-spoof, regression, sec-moderate})

52 Branch
mozilla53
csectype-spoof, regression, sec-moderate
Points:
---
Bug Flags:
sec-bounty +
in-testsuite +
qe-verify +

Firefox Tracking Flags

(firefox50 wontfix, firefox51 wontfix, firefox52 verified, firefox53 verified)

Details

(Whiteboard: [post-critsmash-triage][adv-main52+])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8816351 [details]
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
(Reporter)

Comment 1

2 years ago
Created attachment 8816363 [details]
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.
(Reporter)

Comment 2

2 years ago
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?
(Reporter)

Comment 3

2 years ago
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

2 years ago
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?
Group: firefox-core-security → core-security
Component: Untriaged → DOM
Flags: needinfo?(qab)
Flags: needinfo?(amarchesini)
Product: Firefox → Core
(Reporter)

Comment 5

2 years ago
> 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.
Flags: needinfo?(qab)
(Assignee)

Updated

2 years ago
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
(Assignee)

Comment 6

2 years ago
Created attachment 8816920 [details] [diff] [review]
bloburl.patch
Attachment #8816920 - Flags: review?(bugs)
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.
Attachment #8816920 - Flags: review?(bugs) → review-
(Assignee)

Comment 8

2 years ago
Created attachment 8817260 [details] [diff] [review]
bloburl.patch
Attachment #8816920 - Attachment is obsolete: true
Attachment #8817260 - Flags: review?(valentin.gosu)
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.
Attachment #8817260 - Flags: review?(valentin.gosu) → review+
(Assignee)

Comment 10

2 years ago
Test attached to the patch before landing it.
https://hg.mozilla.org/mozilla-central/rev/cb095e4247e2
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 12

2 years ago
I guess we want this uplifted, given 50 is affected?
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
To 51 and 52. Too late for 50 without a rebuild.
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).
Flags: needinfo?(amarchesini)
(Assignee)

Comment 15

2 years ago
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.
Flags: needinfo?(amarchesini)
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.
Group: core-security → core-security-release
Keywords: csectype-spoof, sec-moderate
Baku, it seems that we want to uplift it, could you fill the uplift request to aurora & beta? Thanks
status-firefox50: affected → wontfix
Flags: needinfo?(amarchesini)
(Assignee)

Comment 18

2 years ago
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
Flags: needinfo?(amarchesini)
Attachment #8817260 - Flags: approval-mozilla-aurora?
Comment on attachment 8817260 [details] [diff] [review]
bloburl.patch

OK to uplift to 52 aurora. 
baku, do you need to rebase for beta uplift?
Flags: needinfo?(amarchesini)
Attachment #8817260 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox52: fixed → affected
(Assignee)

Comment 22

2 years ago
Bug 1310483 must be uplift as well otherwise nsIURIWithQuery is not available.
Flags: needinfo?(amarchesini)
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.
status-firefox51: affected → wontfix
https://hg.mozilla.org/releases/mozilla-aurora/rev/1b0e83800dc6
status-firefox52: affected → fixed
Flags: in-testsuite+
Flags: sec-bounty?
Blocks: 1282407
Flags: sec-bounty? → sec-bounty+
Keywords: regression
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main52+]
Alias: CVE-2017-5415
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
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.