User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 By dragging a javascript url to the address bar of another firefox window, we can execute script in the context of any site Reproducible: Always Steps to Reproduce: (see http://greyhatsecurity.org/vulntests/more/jscriptgo.htm) Actual Results: cross site scripting Expected Results: Javascript urls should not be draggable.

i rely on being able to do this

I don't rely on being able to do this. Timeless, can you elaborate?

(In reply to comment #2) > Timeless, can you elaborate? In particular, why isn't cut-n-paste (or "Copy Link Location" and paste) good enough? Yes, it will be slightly more effort for an action that no normal user would ever do (bookmarklets are leading-edge normal, but are bookmarked and not dragged). For some reason the linked test case isn't working for me in 1.0.3 -- maybe one of my extensions or non-standard pref settings is interfering. The "this link" bit is appended to the javascript: url giving a syntax error. Doesn't take much to fix it though. This doesn't seem to work in the Suite -- when you drag to the urlbar it's populated but not automatically executed. The user would have to see the odd javascript: url and think it's a good idea to execute it. Some would, probably, but that much less a problem. Dragging to the 'go' button is equivalent, and avoids any problems with dragging into the middle of the url bar ending up inserting text (sometimes--why the inconsistency?) rather than replacing it.

Ben, mconnor: how do you want to handle this? One approach would be to make Firefox behave like the suite and just paste, not paste and submit. Or only submit if it's an http(s) url. The other approach is to block drops of javascript: and data: urls entirely (and do so using CheckLoadURI() so we don't have reopens like bug 290949)

If the paste behaviour is considered sufficient, lets do that, and disable entirely on the Go button, to match the behaviour implemented for tabs in Bug 280056. I can write the patch, or you can write and I'll review, your call. :)

The URL is 404. Does anyone have a copy of the testcase they can send me?

mconnor: can you write the patch you mention in comment 5?

This makes drag and drop to the URL bar or the Go button behave as follows: non-URIs and data/javascript URIs will paste on drop in the URL bar, but will not be executed. Dropping on the Go button will have no effect. valid non-data/javascript URIs will continue to function as before. This is the minimum amount of real change, but it seems adequate to the task at hand.

Dragged chrome:// URLs should also be blocked or treated like dragged javascript: URLs.

Comment on attachment 192801 [details] [diff] [review] fix > > // XXXBlake Workaround caret crash when you try to set the textbox's value on dropping >- setTimeout(function(u) { gURLBar.value = u; handleURLBarCommand(); }, 0, url); >+ function urlbarDrop(u) { >+ try { >+ gURLBar.value = u; >+ var uri = makeURI(gURLBar.value); >+ const secMan = Components.classes["@mozilla.org/scriptsecuritymanager;1"] >+ .getService(Components.interfaces.nsIScriptSecurityManager); >+ const nsIScriptSecMan = Components.interfaces.nsIScriptSecurityManager; >+ secMan.checkLoadURI(gBrowser.currentURI, uri, nsIScriptSecMan.DISALLOW_SCRIPT_OR_DATA); >+ handleURLBarCommand(); >+ } catch (ex) {} >+ } >+ setTimeout(urlbarDrop, 0, url); Leave the XXXBlake comment right above the setTimeout call, I think. And please make sure that there's a bug on file for the crash case, and mention it there as well. >+ var url = xferData[0] ? xferData[0] : xferData[1]; >+ if (url) { > getBrowser().dragDropSecurityCheck(aEvent, aDragSession, uri); >- >- loadURI(uri, null, null); >+ try { >+ var uri = makeURI(url); >+ const secMan = Components.classes["@mozilla.org/scriptsecuritymanager;1"] >+ .getService(Components.interfaces.nsIScriptSecurityManager); >+ const nsIScriptSecMan = Components.interfaces.nsIScriptSecurityManager; >+ secMan.checkLoadURI(gBrowser.currentURI, uri, nsIScriptSecMan.DISALLOW_SCRIPT_OR_DATA); >+ loadURI(uri, null, null); >+ } catch (ex) {} > } Lose the if (url) test and just put that whole chunk into the try.

(In reply to comment #9) > Dragged chrome:// URLs should also be blocked or treated like dragged > javascript: URLs. As far as I can tell, they already are, even without this patch.

> + secMan.checkLoadURI(gBrowser.currentURI, uri, nsIScriptSecMan.DISALLOW_SCRIPT_OR_DATA); gBrowser.currentURI will be wrong if you drag from one window to another, or if you switch tabs while dragging.

turns out, whatever crash Blake was hitting way back in 2002 is gone now.

Fixed, branch and trunk.

fyi, fails in firefox 1.0.8/winxp/20060221 (and yes I know it is marked as only fixed on 1.8/trunk).