1321619 - (CVE-2018-5111) URL bar spoof using drag and drop (dnd) and unicode-lookalike protocol to drag plaintext that "looks like" a valid URL

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Mario Gomes]

Attachment: poc.html

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

Steps to reproduce:

hey,

there's a url bar spoof vulnerability that occurs when a specially crafted url is dragged into the address bar.

Steps to reproduce:
1. Open poc.html(attached).
2. Drag and Drop the text in the page into the address bar.
3. notice that now is shown "https://www.google.com", even though the page hasn't been loaded.

cheers,
Mario.

Comment 1

[Mario Gomes]

Attachment: screenshot.png

screenshot demonstrating the vulnerability.

Comment 2

[Mario Gomes]

Is this being ignored?

Comment 3

[:Gijs (he/him)]

(In reply to Mario Gomes from comment #2)
> Is this being ignored?

No. All of Mozilla is in conference this week, and will be (and traveling) until the begining of next week, so that impacts people being available to work/triage this.

From a quick look, this doesn't look like a complete spoof (security info (lock) isn't changed) and so this is sec-moderate or lower, so it isn't a "drop everything and fix this" kind of bug.

I also believe other patches I have in flight already might address this bug, but due to the above I just haven't had cycles to check that.

Comment 4

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #3)
> I also believe other patches I have in flight already might address this
> bug, but due to the above I just haven't had cycles to check that.

This is no longer broken on today's nightly. I suspect my belief was right and that my fix for bug 791597 fixed it. I think I'd like to poke at this more though, feels like we should do more checks on the URI before even attempting to load it (and put it in the URL bar).

Comment 5

[:Gijs (he/him)]

Oops, sorry for bugspam.

Comment 6

[Daniel Veditz [:dveditz]]

Giving it the rating for bug 791597, but it's probably a dupe of (and fixed by) that one.

Comment 7

[Mario Gomes]

Any updates on this?

Comment 8

[:Gijs (he/him)]

(In reply to Mario Gomes from comment #7)
> Any updates on this?

Any updates will appear in the bug. This is still on my list, but I'm on holiday right now. I don't believe this is critical - your testcase requires substantial user action and does not spoof the lock icon (so it's not a complete URL bar spoof). In that sense it's even less severe than 791597, which was sec-moderate.

The testcase doesn't currently work at all on Nightly, but I'm not sure if that will still be the case after I fix bug 1324410. I don't have time to test this right now (as I said, I'm on holiday). I'll look at it again after I get back, update the patch there and get it to land.

Comment 9

[Mario Gomes]

(In reply to :Gijs (gone until 3 jan) from comment #8)
> (In reply to Mario Gomes from comment #7)
> I don't believe this is critical - your testcase requires
> substantial user action and does not spoof the lock icon (so it's not a
> complete URL bar spoof). 

Like it would make it any difference in the real world scenario. Quit being so naive.

Comment 10

[Marco Bonardo [:mak]]

Mario, please be respectful of other people and of our bugzilla netiquette. Attacking developers won't bring anything useful to this report.

Comment 11

[Mario Gomes]

(In reply to Marco Bonardo [::mak] from comment #10)
> Mario, please be respectful of other people and of our bugzilla netiquette.
> Attacking developers won't bring anything useful to this report.

When exactly was I disrespectful?

Comment 12

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

Calling people "naive" is rude and unhelpful, Mario. Be respectful, as requested (and you've been asked previously in other bugs so this is an ongoing issue when you get impatient with people).

Comment 13

[:Gijs (he/him)]

I was right in my suspicion that fixing bug 1324410 will make nightly vulnerable again.

I see several avenues we could pursue to fix this:

1) break all dragging to the URL bar. This is what Edge does. (I don't think this is a good idea. Something about the baby and the bathwater.)
2) break dragging plaintext to the URL bar. This is what IE does.
3) always search for non-URL drops. This is what Chrome does.
4) attempt to build some kind of heuristic to determine whether this is an attempt to spoof the user.


I don't think (4) is likely to work. There will always be more ways (specifically, more unicode characters) to spoof things than we can try to work around, and so I'm not particularly interested in trying. We do need to continue to support unicode typing and dragging to the URL bar for people who don't live in the English or other latin-charset-based languages, so I think that's a losing battle.

I would like to propose (3), with fallback to (2) if the user has toggled the hidden keyword.enabled pref, and while simultaneously improving our url fixup (e.g. bug 1181081) and using it explicitly in the dnd code, to mitigate cases where people "almost" drag a URL to the URL bar, like just dragging "google.com" or "/google.com" etc.

This is a change in UX, so I would like some buy-in from the UX team as well as Marco (with whom I've discussed this before now...) before working on this.

Philipp, the TL;DR here is that dragging arbitrary text to the URL bar and leaving it there is perceived to be a spoofing risk, especially because the user can't always control what the text is (websites can manipulate this). While Marco and I have discussed alternative solutions like showing the current page's URL when the URL bar is not focused (rather than the user's last input), that would likely be confusing and potentially break particular usage scenarios (drag text, then type) and/or not really fix the spoofing risk if users don't re-check the URL bar after re-focusing the content. So I listed some avenues above that we could explore. As noted, I would lean towards (3) unless you have strong objections. Please feel free to ping me on IRC (in private) if you have questions or if something is unclear. :-)

Comment 14

[Marco Bonardo [:mak]]

(In reply to :Gijs from comment #13)
> I would like to propose (3), with fallback to (2) if the user has toggled
> the hidden keyword.enabled pref, and while simultaneously improving our url
> fixup (e.g. bug 1181081) and using it explicitly in the dnd code, to
> mitigate cases where people "almost" drag a URL to the URL bar, like just
> dragging "google.com" or "/google.com" etc.

Yep, we need to fixup the text, it would be sad to drop "www.mozilla.org" and run a search cause it's not a complete url.
Chrome is doing something similar, as I previously said, any text is automatically searched, unless it "looks like" a url, then it is fixed up and visited. I don't think there's any situation where they allow you to drop something in the urlbar and it stays there to be further edited. And in spite of this report, it makes sense as a behavior.

The only 2 problems I see:
1. UX change. User can't drag&drop a text, modify it to his likes and THEN start a search or a visit. This feature gets lost. On the other side, it sounds like an edge case, so we could just decide to take the hit.
2. D&D won't have the same behavior as C&P. IIRC we previously discussed this and there was some hint that we don't consider these 2 operations the same, in terms of spoof, cause C&P requires 2 actions and you see what you are copying (unless the web page populates the clipboard for you!). Before implementing a fix very specific to D&D, we should evaluate how much we care about C&P case, if we care a lot, maybe we need a broader approach? That said, if the patch here ends up being "simple" enough, we could take it regardless as a step forward.

Comment 15

[:Gijs (he/him)]

(In reply to Marco Bonardo [::mak] from comment #14)
> 2. D&D won't have the same behavior as C&P. IIRC we previously discussed
> this and there was some hint that we don't consider these 2 operations the
> same, in terms of spoof, cause C&P requires 2 actions and you see what you
> are copying (unless the web page populates the clipboard for you!). Before
> implementing a fix very specific to D&D, we should evaluate how much we care
> about C&P case, if we care a lot, maybe we need a broader approach? That
> said, if the patch here ends up being "simple" enough, we could take it
> regardless as a step forward.

Copy and paste is also more useful (in the location bar) right now than DND is. I paste things without wanting to use "paste and go" immediately, and then edit, also because paste gives you control over the insertion point, which we don't do for DND (even though on some OSes we'll show a cursor in the middle of the text while dragging :-\ ).

Comment 16

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

(In reply to :Gijs from comment #13)
> I was right in my suspicion that fixing bug 1324410 will make nightly
> vulnerable again.
> 
> I see several avenues we could pursue to fix this:
> 
> 1) break all dragging to the URL bar. This is what Edge does. (I don't think
> this is a good idea. Something about the baby and the bathwater.)
> 2) break dragging plaintext to the URL bar. This is what IE does.
> 3) always search for non-URL drops. This is what Chrome does.
> 4) attempt to build some kind of heuristic to determine whether this is an
> attempt to spoof the user.
> 
> 
> I don't think (4) is likely to work. There will always be more ways
> (specifically, more unicode characters) to spoof things than we can try to
> work around, and so I'm not particularly interested in trying. We do need to
> continue to support unicode typing and dragging to the URL bar for people
> who don't live in the English or other latin-charset-based languages, so I
> think that's a losing battle.
> 
> I would like to propose (3), with fallback to (2) if the user has toggled
> the hidden keyword.enabled pref, and while simultaneously improving our url
> fixup (e.g. bug 1181081) and using it explicitly in the dnd code, to
> mitigate cases where people "almost" drag a URL to the URL bar, like just
> dragging "google.com" or "/google.com" etc.
> 
> This is a change in UX, so I would like some buy-in from the UX team as well
> as Marco (with whom I've discussed this before now...) before working on
> this.
> 
> Philipp, the TL;DR here is that dragging arbitrary text to the URL bar and
> leaving it there is perceived to be a spoofing risk, especially because the
> user can't always control what the text is (websites can manipulate this).
> While Marco and I have discussed alternative solutions like showing the
> current page's URL when the URL bar is not focused (rather than the user's
> last input), that would likely be confusing and potentially break particular
> usage scenarios (drag text, then type) and/or not really fix the spoofing
> risk if users don't re-check the URL bar after re-focusing the content. So I
> listed some avenues above that we could explore. As noted, I would lean
> towards (3) unless you have strong objections. Please feel free to ping me
> on IRC (in private) if you have questions or if something is unclear. :-)

Thanks for the summary!
I agree with your proposal of going with 3) -- just tried this in Chrome and that behavior seems fine to me.

Comment 17

[Mario Gomes]

Any updates on this?

Since Gijs claims this doesn't affect Nightly version. Is this a dupe?
Is this going to be fixed? Are there any estimate date for when it's coming out?

Comment 18

[:Gijs (he/him)]

(In reply to Mario Gomes from comment #17)
> Any updates on this?

As I already said in comment #8, updates will be on the bug. Unless you turned email off in bugzilla, you should also be receiving emails from this bug. The last update was a week ago. I'm not sure why you're asking.

> Since Gijs claims this doesn't affect Nightly version.

I pointed out in comment #13 that fixing bug 1324410 would re-break Nightly. As you can see in bugzilla, I fixed bug 1324410, so this is no longer 'fixed' in Nightly - dragging text to the location bar was just buggy in Nightly for a time, which also 'fixed' this bug as long as it was broken.

> Is this a dupe?

No.

> Is this going to be fixed?

Yes.

> Are there any estimate date for when it's coming out?

No.

Comment 19

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

Mario, this is a sec-moderate issue. It isn't an emergency and devs will work on moderate issues as they can to get them fixed. You don't need to ask "is this going to be fixed?" That's a bit insulting.

Comment 20

[Mario Gomes]

hey,

since this isn't being fixed so soon. Has the bug bounty decision been made?

it's complicated working with you guys, everything that I say results in complains. What's the deal? I ask for updates because it seems like the bugs has been forgotten even thought it's sec-moderate it's still a bug and is restrict(if it wasn't so important, it'd open to everybody see).

everything that it's asked seems to be "insulting", didn't know mozilla was *that* sensitive.

Comment 21

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

Mario,

Bugzilla shouldn't be used in place of email. If you have questions, as has been said before, please email [e-mail] and leave the bug for discussions of patches or technical questions or requests. For the hundredth time, please quit asking for "Is there an update" or similar *in the bugs*. If the devs have an update, it will show up here. There is nowhere else for it to appear. If there are no updates here, then there are no updates. All general questions should be sent to us in email, not added to the bug (over and over).

> it's complicated working with you guys, everything that I say results in
> complains. What's the deal?

This is because people find your behavior to often be rude.

> I ask for updates because it seems like the bugs
> has been forgotten even thought it's sec-moderate it's still a bug and is
> restrict(if it wasn't so important, it'd open to everybody see).

Bugs aren't forgotten. We have regular reports of unfixed security bugs. We are aware that they exist. Sec-critical bugs get the highest priority, followed by sec-high bugs. Sec-moderates are fixed as makes sense but they don't necessarily override the priority of other, ongoing, work, and are not considered emergencies.

> everything that it's asked seems to be "insulting", didn't know mozilla was
> *that* sensitive.

It is because of both how you ask and the fact that on just about every bug that you report, you wind up engaging in this behavior. We do notice these things and it becomes tiresome. You've literally interacted with Mozilla for *years* so you should know how our processes work and what we've asked you to do or not do in the past.

Comment 22

[Mario Gomes]

4 months old bug. Is this going to be patched any soon?

Comment 23

[Daniel Veditz [:dveditz]]

Does not spoof the lock icon and requires unconvincing user interaction: not paying a bounty (note, we did not pay for bug 791597 either).

Comment 24

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is a P1 bug without an assignee. 

P1 are bugs which are being worked on for the current release cycle/iteration/sprint. 

If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.

Comment 25

[Panos Astithas (he/him) [:past] (please ni?)]

Assigning to Paolo per our recent discussions.

Comment 26

[:Paolo Amadini]

Attachment: The patch

This executes the default action associated with the text that was dropped on the location bar, which is usually a search. If the "keyword.enabled" preference is false in about:config, this will perform a navigation, rather than disallowing the drop. This is because this preference is hidden, so I went with the simplest approach. I also haven't written an explicit test case for it.

I'll land this patch with the bug number once it's reviewed. I've started a tryserver build here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbddea8e00797642f35270bd5d60b155dd8abe51

Comment 27

[Marco Bonardo [:mak]]

Comment on attachment 8925542 [details] [diff] [review]
The patch

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

Testing the Try build, it seems to be doing exactly what we expect, search or load url on drop. I'd like this to land asap, so we have about one full week of Nightly testing.

::: browser/base/content/test/urlbar/browser_dragdropURL.js
@@ +47,5 @@
>  });
>  
>  add_task(async function checkDragText() {
> +  await BrowserTestUtils.withNewTab(TEST_URL, async browser => {
> +    let promiseSearch = BrowserTestUtils.browserLoaded(browser, false, DRAG_TEXT_URL);

nit: maybe name it promiseLoad? in one case it's not a search

Comment 28

[Marco Bonardo [:mak]]

Please remember to change the commit message to just the bug number and no description.

Comment 29

[:Paolo Amadini]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa132747394e61e607ce2e3bea248c1e66011aea

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aa132747394e