Closed
Bug 1321719
(CVE-2017-5415)
Opened 7 years ago
Closed 7 years ago
Address bar spoof using blob URI scheme
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
VERIFIED
FIXED
mozilla53
People
(Reporter: qab, Assigned: baku)
References
Details
(Keywords: csectype-spoof, regression, sec-moderate, Whiteboard: [post-critsmash-triage][adv-main52+])
Attachments
(3 files, 1 obsolete file)
253 bytes,
text/html
|
Details | |
62.52 KB,
image/png
|
Details | |
905 bytes,
patch
|
valentin
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•7 years ago
|
||
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•7 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•7 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•7 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•7 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•7 years ago
|
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8816920 -
Flags: review?(bugs)
Comment 7•7 years ago
|
||
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•7 years ago
|
||
Attachment #8816920 -
Attachment is obsolete: true
Attachment #8817260 -
Flags: review?(valentin.gosu)
Comment 9•7 years ago
|
||
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•7 years ago
|
||
Test attached to the patch before landing it.
Comment 11•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cb095e4247e2
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 12•7 years ago
|
||
I guess we want this uplifted, given 50 is affected?
Comment 13•7 years ago
|
||
To 51 and 52. Too late for 50 without a rebuild.
Comment 14•7 years ago
|
||
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•7 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)
Comment 16•7 years ago
|
||
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
Comment 17•7 years ago
|
||
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•7 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+
Comment 21•7 years ago
|
||
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
Updated•7 years ago
|
Assignee | ||
Comment 22•7 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.
Comment 24•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/1b0e83800dc6
Flags: in-testsuite+
Updated•7 years ago
|
Flags: sec-bounty?
Updated•7 years ago
|
Updated•7 years ago
|
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
Updated•7 years ago
|
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main52+]
Updated•7 years ago
|
Alias: CVE-2017-5415
Comment 25•7 years ago
|
||
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
Updated•6 years ago
|
Group: core-security-release
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•