718203 - prevent self-XSS in homepage icon (disallow javascript: drops)

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, enhancement)

Description

[Mario Gomes]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:


Tested on Firefox 9.01
Windows 7 SP1

Reproduce:
1. Change you homepage to "javascript:alert(document.cookie)" or also you can be "javascript://www.google.com.br/?xss=%0aalert(1)".
2. Go to http://www.google.com.
3. Click in homepage icon.
4. See yours cookies of "http://www.google.com".

I think this can be exploited to steal cookies. Because extensions can change the homepage, and convence the victim to click in homepage.

Comment 1

[Daniel Veditz [:dveditz]]

Extensions can do anything so as an attack vector this is pretty uninteresting, but it also doesn't make any sense to have a javascript: homepage and we could prevent that.

rather than blacklisting javascript: maybe we could check for schemes that INHERIT the previous page's principal. And rather than blocking it outright we could first load a blank page to clear the scope and then load the URL. Not very interesting for javascript: urls but would preserve the use of data: as a homepage scheme and any future such schemes. If that's not interesting (not sure data: is, you could always store a local file:/// homepage) then just block 'em.

I'm pretty sure we block dropping javascript: on the homepage button as we do with many drop targets. If we don't then we really should.

Comment 2

[Daniel Veditz [:dveditz]]

We should prevent javascript: urls from being dropped on the homepage icon at least, and consider not allowing it as a home page at all (through direct config setting).

This could be used in social engineering attacks similar to the ones that led us to prevent the use of javascript: in the navigation bar.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

untested, for the moment

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 5

[Neil Deakin]

Comment on attachment 591977 [details] [diff] [review]
patch

Please document the extra argument in nsIDroppedLinkHandler.idl

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: with comment

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 593256 [details] [diff] [review]
with comment

sr=me

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: fix for linux drag&drop failure

My test was failing on Linux for some reason. It appears to be because the synthesizeDrop code tries to synthesize mouse moves on the srcElement, but this wasn't working well for the home button (because it has rounded corners, maybe?). Changing that code to use the center point of the srcElement for the initial move made it work.

Comment 9

[Neil Deakin]

Comment on attachment 593986 [details] [diff] [review]
fix for linux drag&drop failure

We conveniently also have synthesizeMouseAtCenter available for this purpose.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

I can't use synthesizeMouseAtCenter for the second event that needs to be slightly offset from the first, so I need to calculate the coordinates anyways - I figured I'd do that just once.

Comment 11

[Neil Deakin]

Comment on attachment 593986 [details] [diff] [review]
fix for linux drag&drop failure

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

OK then. Technically, the specific coordinates of the second mousemove don't matter as long as they are more than the drag threshold, but I think the approach used here would work in more situations.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c58bb4638a4

Comment 13

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/4c58bb4638a4

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Attachment: rolled up branch patch

This is a pre-requisite to the patch for bug 704354. It's been on m-c for 10 days and is relatively low risk (one interface change, but it's an optional parameter to a mostly-internal interface).

Comment 15

[Alex Keybl [:akeybl]]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #14)
> Created attachment 597209 [details] [diff] [review]
> rolled up branch patch
> 
> This is a pre-requisite to the patch for bug 704354. It's been on m-c for 10
> days and is relatively low risk (one interface change, but it's an optional
> parameter to a mostly-internal interface).

Do we expect a nomination for bug 704354 in the Beta 11/12 time periods?

Comment 16

[:Gavin Sharp [email: [e-mail]]]

(In reply to Alex Keybl [:akeybl] from comment #15)
> Do we expect a nomination for bug 704354 in the Beta 11/12 time periods?

Yes - its patch is in bug 724161.

Comment 17

[Alex Keybl [:akeybl]]

Comment on attachment 597209 [details] [diff] [review]
rolled up branch patch

Let's re-nominate this once bug 724161 is nominated, since they appear to be co-dependent (please correct me if I'm wrong).

Comment 18

[:Gavin Sharp [email: [e-mail]]]

(In reply to Alex Keybl [:akeybl] from comment #17)
> Let's re-nominate this once bug 724161 is nominated, since they appear to be
> co-dependent (please correct me if I'm wrong).

They're not co-dependent - this fix can stand on its own, but the fix for bug 724161/bug 704354 depends on this one.

Comment 19

[Alex Keybl [:akeybl]]

Comment on attachment 597209 [details] [diff] [review]
rolled up branch patch

[Triage Comment]
Approving for Aurora/Beta in support of bug 704354. Please land today.

Comment 20

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e7946e3fb9ab
https://hg.mozilla.org/releases/mozilla-aurora/rev/6ff957842178

Comment 21

[Ioana (away)]

Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
BuildID: 20120222074758

The issue still reproduces on Firefox 11.0 beta 4 with the steps from comment 0. Are there any chances the fix didn't get to land in beta 4?

Comment 22

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-beta/log/FIREFOX_11_0b4_RELEASE/content/base/public/nsIDroppedLinkHandler.idl shows that the fix landed in time for 11b4.

The steps in comment 0 don't quite cover what was fixed in this bug, though. What we fixed here was preventing the ability to drag the text of a JavaScript URI like "javacript:1" to the home button to set the home page.

STR for the bug we fixed:
1) select "javascript:1" text in this comment
2) drag and drop it onto the home button in the toolbar

Expected: no dialog, nothing happens
Actual: dialog confirming whether you want to set the page as your home page

Dragging non-javascript: URIs (like "http://google.com") to the home button should still work (show the confirmation dialog).

Comment 23

[Ioana (away)]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0

The issue is also still reproducible with the steps from comment 22, but it's intermittent. 

Windows 7:
The first time I dragged "javascript:1" to the Home button, the dialog confirming whether I wanted to set it as Home page was displayed. I could set it as Home page without any issues. The next ~20 tries it didn't reproduce though, after which it reproduced again.

Mac 10.7:
The issue reproduced from the 3rd try, then it reproduced again after ~10 tries.

Ubuntu 11.10:
The issue reproduced from the 2nd try, then it reproduced again after 15-20 tries.

Comment 24

[:Gavin Sharp [email: [e-mail]]]

Ioana: are you re-selecting the text each time, or dragging the exact same selected text? Are you always dragging from the same source (a page in the content area)?

Comment 25

[Ioana (away)]

Gavin,

I tried to reproduce this issue again today (on Windows 7) and although I tried it more than 30 times, it didn't reproduce.

These are the details from when it reproduced yesterday:
Some of the times I re-selected the text, whilst others I just dragged the already 
selected text. I was always dragging the text from the same source (comments on this page).

Comment 26

[Ioana (away)]

Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0 (20120222074758)

This issue also reproduces this way:
STR1:
1. Enter "javascript:1" in the search bar.
2. Select the text from the search bar and drag it to the Home button.

STR2:
1. Select "javascript:1" from this comment and drag it to the Bookmarks button.
2. Click on the Bookmarks button.
3. Select "javascript:1" and drag it to the Home button.
This case could happen often enough since the Bookmarks button is right next to the Home one and so user might reproduce it by mistake.

Gavin, can you fix these issue here too, or do you prefer I file new issues for them?

Comment 27

[:Gavin Sharp [email: [e-mail]]]

(In reply to Ioana Budnar [QA] from comment #26)
> 1. Enter "javascript:1" in the search bar.
> 2. Select the text from the search bar and drag it to the Home button.

I'll need to look into this case in more detail (in a different bug). The search bar is considered a "privileged" context, but I thought the blocking of javascript: would override that.

> STR2:
> 1. Select "javascript:1" from this comment and drag it to the Bookmarks
> button.
> 2. Click on the Bookmarks button.
> 3. Select "javascript:1" and drag it to the Home button.
> This case could happen often enough since the Bookmarks button is right next
> to the Home one and so user might reproduce it by mistake.

In step #3, are you dragging the bookmark from the bookmark menu on to the home button? If so, this is effectively the same as STR1. It would be great if you could file a new bug on this.

Comment 28

[:Gavin Sharp [email: [e-mail]]]

Going to re-resolve this as FIXED, given comment 25.

Note that if you select "javascript:1 (with a leading quote), then the prompt does appear. This may not be ideal, but it isn't a security issue (since the quote makes the URI invalid).

Comment 29

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/507558a58f6f

Comment 30

[:Gavin Sharp [email: [e-mail]]]

Since bug 652494 only landed for firefox 11, I needed to adjust the test on ESR to fix bustage. I took the opportunity to also include the orange fix from bug 724309 while I was at it.
https://hg.mozilla.org/releases/mozilla-esr10/rev/45c0c93ccc2b

Comment 31

[Ioana (away)]

Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0
BuildID: 20120228210006

Verified again on the above builds. This issue doesn't reproduce anymore. New bugs will be filed for the two new issues from comment 26.

Comment 32

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

(In reply to Gavin Sharp (use [e-mail] for email) from comment #22)
> 
> The steps in comment 0 don't quite cover what was fixed in this bug, though.
> What we fixed here was preventing the ability to drag the text of a
> JavaScript URI like "javacript:1" to the home button to set the home page.
> 
> STR for the bug we fixed:
> 1) select "javascript:1" text in this comment
> 2) drag and drop it onto the home button in the toolbar
> 
> Expected: no dialog, nothing happens
> Actual: dialog confirming whether you want to set the page as your home page


This problem occurs in 1.9.2.27 on XP SP3 as well.

Comment 33

[Ioana (away)]

Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
BuildID: 20120314195616

Mozilla/5.0 (Windows NT 6.1; rv:10.0.3) Gecko/20100101 Firefox/10.0.3
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.3) Gecko/20100101 Firefox/10.0.3
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0.3) Gecko/20100101 Firefox/10.0.3
BuildID: 20120302182214