Last Comment Bug 280056 - When dropping a javascript link to a tab, the script runs in the security context of the site currently displayed in the tab
: When dropping a javascript link to a tab, the script runs in the security con...
Status: VERIFIED FIXED
[sg:fix] need approvals
: fixed-aviary1.0.1, fixed1.4.5, fixed1.7.6
Product: Core
Classification: Components
Component: Drag and Drop (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Daniel Veditz [:dveditz]
:
:
Mentors:
http://www.mikx.de/firetabbing/
Depends on:
Blocks: sbb+ sg-ff101 sg-moz176
  Show dependency treegraph
 
Reported: 2005-01-27 09:28 PST by Michael Krax
Modified: 2007-04-01 14:32 PDT (History)
8 users (show)
caillon: blocking1.7.6+
caillon: blocking‑aviary1.0.1+
asa: blocking‑aviary1.5+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix tabbrowser drops (2.25 KB, patch)
2005-02-02 11:59 PST, Daniel Veditz [:dveditz]
jst: review+
mozilla: superreview+
caillon: approval‑aviary1.0.1+
caillon: approval1.7.6+
Details | Diff | Splinter Review

Description Michael Krax 2005-01-27 09:28:03 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

The javascript security manager usually prevents that a javascript: URL from one
host is opened in a window displaying content from another host. But when the
link is dropped to a tab the security manager does not kick in. Probably it is
treated like the javascript: URL was typed by the user - which also does not
trigger the security manager check. 

Reproducible: Always

Steps to Reproduce:
1. Open http://www.mikx.de/firetabbing/
2. Drop the example links to other tabs

Actual Results:  
The scripts are running in the security context of the site currently displayed
in the tab

Expected Results:  
The script should run in its own security context (e.g. about:blank) or the
javascript security manager should kick in (just like it does when you try a
window.location='javascript:' across different hosts).

Tabbed browsing is a great feature to organize mutliple website, but after a
while also tabs become too much. Now you have two options: Close tabs and open
new ones (CTRL+W to close a tab, followed by a CTRL+click on a link to open a
new one), or just recycle already open tabs by dragging links to them - the
solution i prefer.

Dropping links to tabs containing other websites is a common workflow. Well, at
least to me ;)
Comment 1 Daniel Veditz [:dveditz] 2005-01-28 00:17:13 PST
->Core

Arg! this was supposed to be fixed by bug 250862. Is there something we could do
that's better than hacking each onDrop handler? How many more do we need to
patch? (obviously the one in tabbrowser.xml)

http://lxr.mozilla.org/mozilla/search?string=ondrop
Comment 2 Daniel Veditz [:dveditz] 2005-02-02 11:59:40 PST
Created attachment 173200 [details] [diff] [review]
fix tabbrowser drops

Makes tabbrowser drop restrictions match those added to content area for bug
250862. I checked the other browser drop targets (go button, urlbar, livefeeds,
home button, download button, etc) and they all appear safe to allow
javascript: urls.

Need to check the mail drop targets.
Comment 3 Christopher Aillon (sabbatical, not receiving bugmail) 2005-02-02 13:08:18 PST
Plussing.  We should get this on the branches.
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2005-02-02 13:20:03 PST
Comment on attachment 173200 [details] [diff] [review]
fix tabbrowser drops

r=jst
Comment 5 Ben Bucksch (:BenB) 2005-02-03 09:02:02 PST
on public website, known by press, opening.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2005-02-03 10:30:50 PST
Comment on attachment 173200 [details] [diff] [review]
fix tabbrowser drops

sr=me, fwiw
Comment 7 Christopher Aillon (sabbatical, not receiving bugmail) 2005-02-03 13:35:47 PST
Comment on attachment 173200 [details] [diff] [review]
fix tabbrowser drops

a=caillon for both branches.
Comment 8 Daniel Veditz [:dveditz] 2005-02-05 01:18:33 PST
Fixed on trunk plus 1.7 and aviary1.0.1 branches
Comment 9 Michael Krax 2005-02-07 06:48:04 PST
Since the bug is fixed i will make it public tonight. Just to let you now
beforehand.
Comment 10 Ben Bucksch (:BenB) 2005-02-07 15:35:45 PST
Unhiding <http://www.heise.de/newsticker/meldung/56140>
Comment 11 HJ 2005-02-11 08:18:49 PST
Killing ondrop for javascript: isn't a real solution, it seems more like a quick
hack to fix a possible security issue. There must be a better way to solve this
issue, without killing ondrop completely.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2005-02-11 10:40:46 PST
Why does that not seem like a real fix?
Comment 13 HJ 2005-02-12 16:09:57 PST
(In reply to comment #12)
> Why does that not seem like a real fix?

You should still be able to drag javascript: links to new tabs, or the tab bar,
but that no longer works with this 'fix', right?
Comment 14 Juha-Matti Laurio 2005-02-18 15:12:11 PST
This was (is) assigned as Secunia ID 14160;

http://secunia.com/advisories/14160/
(Mozilla / Firefox Three Vulnerabilities)

marked as "The vulnerabilities have been fixed in the CVS repository" on 8th
February, 2005.
Comment 15 Jay Patel [:jay] 2005-02-22 19:50:30 PST
Verified Fixed on 2/21 builds for Aviary 1.0.1, Mozilla 1.7.6, and both Trunk. 
The testcases at http://www.mikx.de/firetabbing/ no longer allow you to execute
the js in tabs.  

HJ:  I haven't had time to thoroughly test this, so I can't say whether you can
no longer drag javascript: urls to tabs/url bar/etc at all (according to comment
#2, this fix just puts sticter restrictions on cross-host js execution), but
please feel free to test this out in any ways that you are accustomed to using
that feature and see what works for you.

Perhaps someone else can better explain whether any drag n drop of js is still
intact and exactly what those restrictions are.  Bug 250862 provides some clues.

Comment 16 HJ 2005-03-06 02:41:28 PST
Someone should look at this code snip again:

+            if (!url || !url.length || url.indexOf(" ", 0) != -1 ||
+                /^\s*(javascript|data):/.test(url))

1) why do you need \s* if you already have url.indexOf(" ", 0) != -1 ?
2) new tabs/tab bar are save targets, so why did you disable them?
Comment 17 Daniel Veditz [:dveditz] 2005-04-19 12:11:25 PDT
awarded a bug bounty

Note You need to log in before you can comment on or make changes to this bug.