Closed Bug 1269832 Opened 4 years ago Closed 4 years ago

URL spoofing possibility with "show domain only" url bar

Categories

(Firefox for Android :: General, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 49
Tracking Status
firefox46 --- unaffected
firefox47 + verified
firefox48 + verified
firefox49 + verified
firefox-esr45 --- unaffected
fennec 47+ ---

People

(Reporter: kats, Assigned: sebastian)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Attached file spoof.html (obsolete) —
URL spoofing is trivial using javascript: URLs. See attached test case. If you click on the link in fennec, it will show "google.com" in the URL bar and whatever content you please as the page.
Attachment #8748314 - Attachment mime type: text/plain → text/html
Attached file spoof.html
Simpler version, the window.open/close thing was unnecessary and left over from the thing I was initially trying to do.
Attachment #8748314 - Attachment is obsolete: true
tracking-fennec: --- → ?
Flags: needinfo?(s.kaspari)
Ouch! Good you found this.
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Flags: needinfo?(s.kaspari)
This patch limits the new behavior to http/https URLs.
Attachment #8748588 - Flags: review?(margaret.leibovic)
Attachment #8748588 - Flags: review?(bugmail.mozilla)
Comment on attachment 8748588 [details] [diff] [review]
1269832-ToolbarDisplayLayout-http.patch

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

URL schemes are case insensitive, so you probably need to use startsWithIgnoreCase or manually do some case folding, unless the URL is already normalized at that point. But otherwise the patch looks ok to me,  but I don't know the code well enough to know if you missed any spots.
Attachment #8748588 - Flags: review?(bugmail.mozilla) → review+
Attachment #8748319 - Attachment mime type: text/plain → text/html
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> URL schemes are case insensitive, so you probably need to use
> startsWithIgnoreCase or manually do some case folding, unless the URL is
> already normalized at that point.

Yeah, at this point we get the "clean" URL from Gecko. StringUtils as a more general purpose helper could ignore the case but I saw that other methods (stripScheme) don't do it, so I just followed.
Comment on attachment 8748588 [details] [diff] [review]
1269832-ToolbarDisplayLayout-http.patch

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

Good catch, this looks reasonable to me.
Attachment #8748588 - Flags: review?(margaret.leibovic) → review+
Let's make sure to uplift this to 47. I don't think we need to wait for security approval, since this isn't on release yet.
tracking-fennec: ? → 47+
[Tracking Requested - why for this release]: bug related to feature in 47
Duplicate of this bug: 1270874
Comment on attachment 8748588 [details] [diff] [review]
1269832-ToolbarDisplayLayout-http.patch

Approval Request Comment

[Feature/regressing bug #]: Shortened URL introduced in bug 1236431.

[User impact if declined]: See comment 0: Arbitrary domain can be shown in URL bar using JavaScript.

[Describe test coverage new/current, TreeHerder]: Local testing.

[Risks and why]: Low - Showing full URL for non-http/https URLs. We already show the full URL on tablets.

[String/UUID change made/needed]: -
Attachment #8748588 - Flags: approval-mozilla-beta?
Attachment #8748588 - Flags: approval-mozilla-aurora?
Comment on attachment 8748588 [details] [diff] [review]
1269832-ToolbarDisplayLayout-http.patch

Recent regression, Aurora48+, Beta47+
Attachment #8748588 - Flags: approval-mozilla-beta?
Attachment #8748588 - Flags: approval-mozilla-beta+
Attachment #8748588 - Flags: approval-mozilla-aurora?
Attachment #8748588 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/8fe0095972a5
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Group: firefox-core-security → core-security-release
Verified as fixed on Firefox 47 Beta 6, latest Aurora and latest Nightly using the "spoof.html" test case
Blocks: 1236431
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.