Closed Bug 718203 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox :: Tabbed Browser, enhancement)

9 Branch
enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 13
Tracking Status
firefox10 --- wontfix
firefox11 --- verified
firefox12 --- verified
firefox-esr10 --- verified
status1.9.2 --- wanted

People

(Reporter: netfuzzerr, Assigned: Gavin)

References

Details

(Whiteboard: [sg:low social engineering][qa!])

Attachments

(3 files, 2 obsolete files)

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.
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.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: XSS in homepage icon. → prevent self-XSS in homepage icon.
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.
Component: Untriaged → Tabbed Browser
QA Contact: untriaged → tabbed.browser
Summary: prevent self-XSS in homepage icon. → prevent self-XSS in homepage icon (disallow javascript: drops)
Whiteboard: [sg:low social engineering]
Attached patch patch (obsolete) — Splinter Review
untested, for the moment
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review
Attachment #591678 - Attachment is obsolete: true
Attachment #591977 - Flags: review?
Attachment #591977 - Flags: review? → review?(enndeakin)
Comment on attachment 591977 [details] [diff] [review]
patch

Please document the extra argument in nsIDroppedLinkHandler.idl
Attachment #591977 - Flags: review?(enndeakin) → review+
Attached patch with commentSplinter Review
Attachment #591977 - Attachment is obsolete: true
Attachment #593256 - Flags: superreview?(bzbarsky)
Comment on attachment 593256 [details] [diff] [review]
with comment

sr=me
Attachment #593256 - Flags: superreview?(bzbarsky) → superreview+
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.
Attachment #593986 - Flags: review?(enndeakin)
Comment on attachment 593986 [details] [diff] [review]
fix for linux drag&drop failure

We conveniently also have synthesizeMouseAtCenter available for this purpose.
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 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.
Attachment #593986 - Flags: review?(enndeakin) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c58bb4638a4
Flags: in-testsuite+
OS: Windows 7 → All
Hardware: x86 → All
Target Milestone: --- → Firefox 13
No longer blocks: CVE-2012-0455
http://hg.mozilla.org/mozilla-central/rev/4c58bb4638a4
Group: core-security
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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).
Attachment #597209 - Flags: approval-mozilla-beta?
Attachment #597209 - Flags: approval-mozilla-aurora?
(In reply to Gavin Sharp (use gavin@gavinsharp.com 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?
(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 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).
Attachment #597209 - Flags: approval-mozilla-beta?
Attachment #597209 - Flags: approval-mozilla-beta-
Attachment #597209 - Flags: approval-mozilla-aurora?
Attachment #597209 - Flags: approval-mozilla-aurora-
(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.
Attachment #597209 - Flags: approval-mozilla-beta?
Attachment #597209 - Flags: approval-mozilla-beta-
Attachment #597209 - Flags: approval-mozilla-aurora?
Attachment #597209 - Flags: approval-mozilla-aurora-
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.
Attachment #597209 - Flags: approval-mozilla-beta?
Attachment #597209 - Flags: approval-mozilla-beta+
Attachment #597209 - Flags: approval-mozilla-aurora?
Attachment #597209 - Flags: approval-mozilla-aurora+
Whiteboard: [sg:low social engineering] → [sg:low social engineering][qa+]
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?
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).
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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)?
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).
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?
(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.
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).
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Attachment #597209 - Flags: approval-mozilla-esr10?
Attachment #597209 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
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
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.
Whiteboard: [sg:low social engineering][qa+] → [sg:low social engineering][qa+][qa!:11]
(In reply to Gavin Sharp (use gavin@gavinsharp.com 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.
Blocks: 732413
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
Status: RESOLVED → VERIFIED
Whiteboard: [sg:low social engineering][qa+][qa!:11] → [sg:low social engineering][qa!]
Depends on: 777571
You need to log in before you can comment on or make changes to this bug.