Bug 1321719 (CVE-2017-5415)

Address bar spoof using blob URI scheme

VERIFIED FIXED in Firefox 52

Status

()

defect
VERIFIED FIXED
3 years ago
3 months ago

People

(Reporter: qab, Assigned: baku)

Tracking

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

52 Branch
mozilla53
Points:
---
Dependency tree / graph
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

3 years ago
Posted file 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

3 years ago
Posted image 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

3 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

3 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

3 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

3 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

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

Comment 6

3 years ago
Posted patch bloburl.patch (obsolete) — Splinter Review
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

3 years ago
Posted patch bloburl.patchSplinter Review
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

3 years ago
Test attached to the patch before landing it.
https://hg.mozilla.org/mozilla-central/rev/cb095e4247e2
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 12

3 years ago
I guess we want this uplifted, given 50 is 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

3 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
Baku, it seems that we want to uplift it, could you fill the uplift request to aurora & beta? Thanks
Flags: needinfo?(amarchesini)
Assignee

Comment 18

3 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+
Assignee

Comment 22

3 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.
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
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.