1224910 - Potential address bar spoofing by using invalid URL scheme

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Muneaki Nishimura]

Attachment: screenshot when the issue reproduced

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

Steps to reproduce:

1. Open a new window and navigate it to an URL that has invalid URL scheme like nttps://google.com
2. Location bar shows the invalid URL nttps://google.com, but its browsing context is inherited from the opener.

A proof of concept code is below.
http://mallory.csrf.jp/ios/spoofing2.html


Actual results:

JavaScript code running on the opener can inject any HTML to the new window without changing the URL in address bar. I think it can be used for address bar spoofing attack.



Expected results:

Firefox should show an invalid scheme error screen instead of about:blank.

Comment 1

[Daniel Veditz [:dveditz]]

Not sure if the iOS version is part of the bounty program but nominating all the same

Comment 2

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

Does Safari on iOS have the same issue?

Comment 3

[Muneaki Nishimura]

Attachment: Test result on Safari for iOS

It's not reproducible on Safari.
As attached the location bar in a new window shows a message "Search or enter website name" but not the malicious URL given from the parent window.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Confirmed that this is a bug we will address in Firefox. Thank you for finding this one.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/1300

There are two related fixes in this patch:

• Better handle invalid schemes opened in popup windows. For example `foo://lalala.com` does now open and show in the location bar.
• Make use of the new `WKFrameInfo.securityOrigin` to show better detailed titles in JavaScript originated alerts. (Via the `WKWebView` delegate)

The second is as a result of the first. With the first now allowing `javascript:` URLs resulted in a crash that is fixed by the second fix.

Comment 6

[Brian Nicholson (:bnicholson)]

Second commit makes sense, but I think we can do better for the fix. I think it's valid to open other types of URIs (data URIs or even empty URIs, for example).

It sounds like the issue is that we fail to load the invalid URL, so the opening page has privileges to modify the opened frame's DOM (fitting into the "empty URI" case mentioned above). If we can, I think a good fix would be to show error pages for invalid schemes like we do with other invalid URLs. If we load an error page, it will be served from a localhost URL, failing the SOP, meaning the opening page shouldn't be able to touch the DOM. As a bonus, this would be a UX improvement since we'd get visual feedback when trying to load invalid pages, as opposed to simply nothing happening.

Comment 7

[Brian Nicholson (:bnicholson)]

FWIW, the fix I'm proposing is essentially what we do on Firefox desktop: the test page opens a new tab, showing the invalid URL in the URL bar, with an error page shown as the content.

Comment 8

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Couple questions/notes:

What would an "invalid scheme" be for popup windows? Can we limit that to a list of known schemes?

I like the idea of presenting an error page instead of simply not creating a new window. That should be easy. And it solves a SOP problem.

This original bug is about popup windows. And I think that is what should be fixed here. But I just noticed that opening nttp://google.com in a new tab does not result in any action at all, so I think a similar fix should happen in a separate bug for normal new tabs.

Comment 9

[Brian Nicholson (:bnicholson)]

(In reply to Stefan Arentz [:st3fan] from comment #8)
> What would an "invalid scheme" be for popup windows? Can we limit that to a
> list of known schemes?

I guess I was hoping that there was some WKWebView callback that was triggered in response to loading invalid URIs/schemes rather than trying to preemptively detect them. Do no error callbacks at all get fired when this happens?

Comment 10

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I set breakpoints on the didFail and didFailProvisionally and they were not triggered on invalid URLs.

Also webView(webView: decidePolicyForNavigationAction: decisionHandler:) is not being called, so we can't intercept it there.

That is why the decision logic lives in the delegate method that creates the new tab. So at that point I think we do need a whitelist of schemes?

Comment 11

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

From last patch:

There are two related fixes in this patch:

• Better handle invalid schemes opened in popup windows. If the scheme is not in our whitelist, we consider it invalid and open a new tab without a context (WKConfiguration) that links to the parent window. After that `webView:decidePolicyForNavigationAction:` will be called and will show an appropriate error message that the URL cannot be opened.
• Make use of the new `WKFrameInfo.securityOrigin` to show better detailed titles in JavaScript originated alerts. (Via the `WKWebView` delegate)

The second is as a result of the first. With the first now allowing `javascript:` URLs resulted in a crash that is fixed by the second fix.

Comment 12

[Brian Nicholson (:bnicholson)]

Left comment in PR. With error pages, I don't think we need the configuration hack (or at least I hope not since cross-origin page manipulations would be a severe vulnerability!).

Comment 13

[Brian Nicholson (:bnicholson)]

Comment on attachment 8691506 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/1300

LGTM with hack removed as mentioned in the PR.

Comment 14

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Addressed the 'configuration hack'. Tested this against:

http://mallory.csrf.jp/ios/spoofing2.html (invalid scheme in url)
http://stefan.arentz.ca/t/window.html (javascript url)
http://stefan.arentz.ca/t/window2.html (invalid domain)

Outcome is good. Although we can handle javascript: links better. Will file followup for that.

Comment 15

[Richard Newman [:rnewman]]

Comment on attachment 8691506 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/1300

This landed:

1a417ed

Comment 16

[Richard Newman [:rnewman]]

Uplifted: b48cd6930140d76bbf1123e164e29a811ec7e99a