Closed Bug 1018154 Opened 10 years ago Closed 10 years ago

Strip "javascript:" on paste instead of disallowing javascript: URLs entered into the location bar from inheriting the current page's principal

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal
Points:
8

Tracking

()

VERIFIED FIXED
Firefox 33
Iteration:
33.2
Tracking Status
firefox33 --- verified

People

(Reporter: dao, Assigned: Gijs)

References

(Depends on 1 open bug)

Details

(Whiteboard: [parity-Chrome][parity-IE])

Attachments

(3 files, 5 obsolete files)

(In reply to Peter Kasting from bug 680302 comment #31)
> I realize that since it's been years since I worked on Firefox' address bar
> my opinions don't count for much, but as the Chrome omnibox owner, I
> strongly agree with Brendan.  There shouldn't be a pref to configure this. 
> Firefox' address bar should simply behave like Chrome's and aim to prevent
> pasted XSS but not manually-typed JS URLs.  We've been shipping this in
> Chrome for some time now and it seems to be sufficiently effective.
> 
> Firefox' current behavior breaks something that has worked for a long time
> in most browsers, and still works everywhere else.  While I conceded that
> the developer console can be used instead, as a non-web developer I don't
> even know the keyboard shortcut for the console offhand, whereas I use
> javascript: URLs in the address bar frequently for very simple tests -- this
> week I was trying to do an alert("Foo") to see whether different browsers
> allowed selection within the text of alert boxes.  Because javascript: URLs
> in Firefox not only don't work, but fail completely silently, I retyped my
> input multiple times (suspecting typos) before realizing that it would never
> work.  In Chrome, by contrast, I get feedback instantly if I paste, before
> I've even hit enter, and can certainly figure out the issue after the fact.
> 
> There's no reason to add a pref here when other browsers demonstrate a
> solution that is sufficiently effective without breaking as many use cases. 
> Prefs also don't help users (like me) who don't know what obscure config
> options for Firefox exist, so they don't solve the problem sufficiently
> anyway.  Finally, the code to strip "javascript:" on paste is easy and
> results in a better UI, compared to the much harder idea of distinguishing
> pasted and typed input, which in the limit isn't feasible (paste + further
> typed editing).

(In reply to :Gavin Sharp from bug 680302 comment #32)
> I'd be open to exploring the "strip javascript: on paste" behavior as a
> replacement for our current behavior, but I don't think it's a high priority.
Whiteboard: [parity-Chrome][parity-IE]
I'd rather not. There's lots of room for the address bar to be actively misleading (because users understand it only as a way to navigate, or by the paste having lots of spaces at the end). See my earlier comments, bug 527530 comment 57 and bug 680302 comment 20.
Your first comment suggests that you think IE and Chrome's protections will be largely ineffective.  That's not proven true in practice.  There have been sporadic attempts to convince people to effectively "type j and paste avascript", but they haven't gained traction.  In practice, IE and Chrome's solution has worked fine.  So I don't find your first comment compelling.

Your second comment is merely a reference to your first comment, plus some stuff that doesn't apply to this bug about prefs.

So I don't find your previous comments to be a compelling argument against this.

Regarding paste having lots of spaces at the end -- this is easily dealt with, and in fact Chrome does deal with this, by trimming leading and trailing whitespace on the pasted string.  We have never to my knowledge had a user complain about this, and it's a small but nice win even for non "javascript:" paste content.
(In reply to Peter Kasting from comment #2)
> There have  been sporadic attempts to convince people to effectively "type j and paste avascript", but they haven't gained traction.

FWIW, at least in IE11, this is nutered by their protection - it appears to check the overall value after every paste and strip "javascript:" off the front if it exists. You have to paste "avascript" and then go to the start and add the "j".

Also, IE strips the leading and trailing whitespace from the value on the clipboard similarly to Chrome.
Right, it would be trivial to make Chrome's rules slightly more aggressive if need be.  Disallowing all forms of javascript: URLs from the address bar isn't necessary.
> There have been sporadic attempts to convince people to effectively "type j and paste avascript", but 
> they haven't gained traction.

"Just below the threshold of wormability" is not the level of security Firefox should aim for.
I don't think the Chrome or IE teams are any less interested in security than the Firefox team.  What my comment is intended to say is that your argument that this "essentially reopens the security hole" is apparently wrong.  Those browsers aren't being lazy, or sloppy.  They're trying to preserve the usability of a longstanding practice while still securing users.  I don't see any concern in your arguments for anything other than security, yet there's no evidence Firefox users have been made any more secure in trade for a neutered address bar.
The fact that we saw *worms* with one less step is enough evidence for me that Chrome's rule makes too many users vulnerable to self-XSS. If you have some evidence or reason to believe that one-off attacks would be *an order of magnitude* less likely to succeed because of the extra 'j', I'd be interested to hear it.

I explained in bug 527530 comment 57 why I don't think the usability argument for allowing typed javascript: URLs is strong.

"I don't see any concern in your arguments for anything other than security" sounds like a poorly aimed ad-hominem attack.
I don't understand what your first sentence means.  Could you say it in more detail?  I'm keenly interested in understanding your argument since you are clearly saying that Chrome, as it stands today, is putting users at risk, and I don't want to take any such claim lightly.

As for the last sentence, there was no intent to be ad hominem, and I'm sorry if that was how you read my comment -- I was being literal, not sarcastic.  I really read what you are saying as that you are completely concerned about security.  That's not an insult; the Chrome security folks are also worried about security.  As a UI engineer, my goal is to achieve both security and usability.  In order to feel like your arguments sufficiently addressed usability, I'd like to see, for example, a response to the note about this being a longstanding cross-browser capability that was removed, perhaps with data that shows that no significant fraction of users actually does this, or that the relevant users are aware of and equally well-served by some of the alternate UIs you propose using in place of the address bar.  We certainly have anecdotal data from people like the inventor of Javascript and the owner of Chrome's address bar that these solutions are not sufficient, but hard data to the contrary would be extremely useful.

And to be clear, I'm not saying that you're wrong if you can't produce data.  I'm merely giving an example of the sort of concern for usability that I think is warranted.
Facebook suffered "self-XSS worm" attacks back when all browsers ran javascript: URLs from the address bar. 

The frequency of such worms dropped after:

1) Firefox nerfed the feature, 
2) IE and Chrome put up minor roadblocks to pasting javascript: URLs,
3) Facebook found a way to distinguish "viral-as-in-virus" from "viral-as-in-popular", and
4) Some of the more susceptible users learned not to XSS themselves (or at least learned that the promised "secret Facebook features" were not real).

There's some threshold fraction of users, |t|, that a worm needs to be able to affect in order to spread exponentially. A very simple model might predict t = avg(1 / number of friends): the worm spreads if at least one of your friends, on average, is susceptible to it.

We don't know which mitigations were most important, and we don't know how much the worm's contagion dropped. We only know it went from being above |t| to being below |t|.
If we truly have no idea how to evaluate the mitigations, then saying definitively that Firefox' behavior is much more secure for users than Chrome's seems unwarranted.  If anything, it seems like we should gather more data so we can determine whether browsers ought to standardize on Firefox' behavior or Chrome's.

That said, given that IE and Chrome together make up the lion's share of the browser market, it seems unlikely that (1) was the critical factor in stopping such worms -- surely the density of non-Firefox users is still high enough to support effective worm propagation.  That supports a hypothesis that (2)-(4) together -- with it not clear which ones were most important -- were sufficient to stop these kinds of worms.

Given this, I still think it would be preferable for Firefox to match other browsers here.  If somehow the above reasoning is wrong, and we begin to see effective social engineering attacks against users, that's valuable information that _all_ browsers, not just Firefox, need to be more aggressive about stopping these.
Boris and I want to get rid of the Sandboxing code in bug 1018583, and gavin wants to fix this bug to do that. Gijs is willing to write the patch, but will chat briefly with gavin about it first. :-)
Blocks: 1018583
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Bobby Holley (:bholley) from comment #11)
> Boris and I want to get rid of the Sandboxing code in bug 1018583, and gavin
> wants to fix this bug to do that. Gijs is willing to write the patch, but
> will chat briefly with gavin about it first. :-)

I discussed this with gavin and will pick this up next week. Marco, can you add this to the *next* iteration? (I'll take adding it to this one, I guess, but I'd be extremely surprised if it was landed and verified by Tuesday)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(mmucci)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: firefox-backlog+
Whiteboard: [parity-Chrome][parity-IE] → [parity-Chrome][parity-IE] p=8 [qa+]
I added it to the priority backlog for 33.2
Flags: needinfo?(mmucci)
If we're removing the ability to "sandbox" javascript: URLs, I think we should not run them at all from the address bar.
I disagree; I think the mitigation proposed here is likely to be sufficient.
Points: --- → 8
QA Whiteboard: [qa+]
Whiteboard: [parity-Chrome][parity-IE] p=8 [qa+] → [parity-Chrome][parity-IE]
Iteration: --- → 33.2
Attached patch WIP patch (obsolete) — Splinter Review
So here's a first attempt. AIUI this basically needs to remove the inherit_security_principal flags as used here, and then disable pasting questionable URLs. That's what this patch does. I believe there are several tests that will need updating after this change, and there should probably be a new test for the stripping behaviour, but I would like to get feedback on the approach before I work on those issues...
Attachment #8447107 - Flags: feedback?(gavin.sharp)
Attachment #8447107 - Flags: feedback?(dao)
Comment on attachment 8447107 [details] [diff] [review]
WIP patch

>         cxmenu.addEventListener("popupshowing", function() {
>           if (!pasteAndGo)
>             return;
>           var controller = document.commandDispatcher.getControllerForCommand("cmd_paste");
>           var enabled = controller.isCommandEnabled("cmd_paste");
>+          if (enabled) {
>+            //XXXgijs Should make sure paste & go isn't enabled for JS url
>+          }

Since you can both paste and submit manually, paste & go shouldn't be disabled.

>+              let pastedURL, pastedCombinedURL;

one declaration per line

>+              let oldStart = oldValue.substring(0, this.inputField.selectionStart);

If oldStart isn't empty, I think you can bail out here. For the case of oldStart being a protocol, see my later comment on pasting something and then typing "javascript:". If oldStart isn't a protocol, there's no risk either unless the user modifies the location bar further.

>+              // Don't allow pasting in full URIs which inherit the security
>+              // context.
>+              if (pastedURL && Services.netutil.URIChainHasFlags(pastedURL, kSecCtxFlag)) {
>+                newValue = oldStart + pastedURL.path + oldEnd;
>+                newCursorPos = oldStart.length + pastedURL.path.length;
>+              }

This looks like it can be fooled by copying and pasting "javascript:javascript:...".

>+              // or pasting in bits that combine to create such a URL:

Why should we prevent this? We don't prevent pasting and then typing "javascript:" either. The case you're trying to cover here doesn't seem significantly more likely to be exploited. Disabling it seems unnecessarily annoying for people who know what they're doing trying to circumvent the protection.
Attachment #8447107 - Flags: feedback?(dao)
(In reply to Dão Gottwald [:dao] from comment #17)
> Comment on attachment 8447107 [details] [diff] [review]
> WIP patch
> 
> >         cxmenu.addEventListener("popupshowing", function() {
> >           if (!pasteAndGo)
> >             return;
> >           var controller = document.commandDispatcher.getControllerForCommand("cmd_paste");
> >           var enabled = controller.isCommandEnabled("cmd_paste");
> >+          if (enabled) {
> >+            //XXXgijs Should make sure paste & go isn't enabled for JS url
> >+          }
> 
> Since you can both paste and submit manually, paste & go shouldn't be
> disabled.

I don't follow what you mean here. AFAICT if you used paste & go here, it would try to 'go' to the modified input, which might end up on a search engine or might just give you an error page, depending on what JS you were copying. Neither of those would be expected, I would have thought, so I think we should disable it rather than not doing what the user would expect.

> 
> >+              let pastedURL, pastedCombinedURL;
> 
> one declaration per line
> 
> >+              let oldStart = oldValue.substring(0, this.inputField.selectionStart);
> 
> If oldStart isn't empty, I think you can bail out here. For the case of
> oldStart being a protocol, see my later comment on pasting something and
> then typing "javascript:". If oldStart isn't a protocol, there's no risk
> either unless the user modifies the location bar further.
> 
> >+              // Don't allow pasting in full URIs which inherit the security
> >+              // context.
> >+              if (pastedURL && Services.netutil.URIChainHasFlags(pastedURL, kSecCtxFlag)) {
> >+                newValue = oldStart + pastedURL.path + oldEnd;
> >+                newCursorPos = oldStart.length + pastedURL.path.length;
> >+              }
> 
> This looks like it can be fooled by copying and pasting
> "javascript:javascript:...".

Yes. Not entirely sure how to fix that, short of a while loop to strip the protocol and defer to the path every time... just creating a URI out of this string doesn't QI to nsINestedURI. :-\

> >+              // or pasting in bits that combine to create such a URL:
> 
> Why should we prevent this? We don't prevent pasting and then typing
> "javascript:" either. The case you're trying to cover here doesn't seem
> significantly more likely to be exploited. Disabling it seems unnecessarily
> annoying for people who know what they're doing trying to circumvent the
> protection.

See comment #3. Essentially, parity-ie. I don't really feel super-strongly here, I guess.

Generally, I am most concerned about removing the LOAD_FLAGS_DISALLOW_INHERIT_OWNER passing. Does that look OK?
Flags: needinfo?(dao)
(In reply to :Gijs Kruitbosch from comment #18)
> > >           if (!pasteAndGo)
> > >             return;
> > >           var controller = document.commandDispatcher.getControllerForCommand("cmd_paste");
> > >           var enabled = controller.isCommandEnabled("cmd_paste");
> > >+          if (enabled) {
> > >+            //XXXgijs Should make sure paste & go isn't enabled for JS url
> > >+          }
> > 
> > Since you can both paste and submit manually, paste & go shouldn't be
> > disabled.
> 
> I don't follow what you mean here. AFAICT if you used paste & go here, it
> would try to 'go' to the modified input, which might end up on a search
> engine or might just give you an error page, depending on what JS you were
> copying. Neither of those would be expected, I would have thought, so I
> think we should disable it rather than not doing what the user would expect.

All of this is also true for pasting and submitting in two steps. Again, I don't see why paste & go should get special treatment here.

> > This looks like it can be fooled by copying and pasting
> > "javascript:javascript:...".
> 
> Yes. Not entirely sure how to fix that, short of a while loop to strip the
> protocol and defer to the path every time...

How about:

s = s.replace(/(\sjavascript:)+/i, "").replace(/(\sdata:)+/i, "");

> > >+              // or pasting in bits that combine to create such a URL:
> > 
> > Why should we prevent this? We don't prevent pasting and then typing
> > "javascript:" either. The case you're trying to cover here doesn't seem
> > significantly more likely to be exploited. Disabling it seems unnecessarily
> > annoying for people who know what they're doing trying to circumvent the
> > protection.
> 
> See comment #3. Essentially, parity-ie. I don't really feel super-strongly
> here, I guess.

I think we can and should drop this.

> Generally, I am most concerned about removing the
> LOAD_FLAGS_DISALLOW_INHERIT_OWNER passing. Does that look OK?

Yes.
Flags: needinfo?(dao)
> s = s.replace(/(\sjavascript:)+/i, "").replace(/(\sdata:)+/i, "");

I forgot ^ in the beginning and \s should have been \s*. Also, this could still be fooled with "data:javascript:". This is better:

s = s.replace(/^(\s*(javascript|data):)+/i, "");
(In reply to Dão Gottwald [:dao] from comment #20)
> > s = s.replace(/(\sjavascript:)+/i, "").replace(/(\sdata:)+/i, "");
> 
> I forgot ^ in the beginning and \s should have been \s*. Also, this could
> still be fooled with "data:javascript:". This is better:
> 
> s = s.replace(/^(\s*(javascript|data):)+/i, "");

I was wondering about that. I'm also worried that this doesn't catch any other protocols which might have the relevant protocol flag, and/or ways in which this regex could be defeated but which would still successfully parse (e.g. null bytes, unicode zero-width spaces, etc.) and be executed. Hence my reliance on the protocol handler info (per bug 656433 comment 12).
(In reply to Dão Gottwald [:dao] from comment #19)
> (In reply to :Gijs Kruitbosch from comment #18)
> > > >           if (!pasteAndGo)
> > > >             return;
> > > >           var controller = document.commandDispatcher.getControllerForCommand("cmd_paste");
> > > >           var enabled = controller.isCommandEnabled("cmd_paste");
> > > >+          if (enabled) {
> > > >+            //XXXgijs Should make sure paste & go isn't enabled for JS url
> > > >+          }
> > > 
> > > Since you can both paste and submit manually, paste & go shouldn't be
> > > disabled.
> > 
> > I don't follow what you mean here. AFAICT if you used paste & go here, it
> > would try to 'go' to the modified input, which might end up on a search
> > engine or might just give you an error page, depending on what JS you were
> > copying. Neither of those would be expected, I would have thought, so I
> > think we should disable it rather than not doing what the user would expect.
> 
> All of this is also true for pasting and submitting in two steps. Again, I
> don't see why paste & go should get special treatment here.

The difference being that you see what happens when you paste separately, before your search provider and URL escaping mangles your input. OTOH, I guess it'll still be on the clipboard so you can re-paste, and it'll be a pain to implement the disabling here, so I can live with leaving that as-is.
Comment on attachment 8447107 [details] [diff] [review]
WIP patch

(I'm unfortunately not going to be able to look into this at all for the next two weeks.)
Attachment #8447107 - Flags: feedback?(gavin.sharp) → feedback?(ttaubert)
Attachment #8447107 - Flags: feedback?(ttaubert)
Attached patch Part 1, v1: code changes (obsolete) — Splinter Review
Attachment #8448679 - Flags: review?(dao)
Attachment #8447107 - Attachment is obsolete: true
Attached patch Part 2, v1: test changes (obsolete) — Splinter Review
These test changes I'm sure are needed based on the dep tree of this bug. I pushed https://tbpl.mozilla.org/?tree=Try&rev=c13b0d4942ef to see if there's anything else.
Attachment #8448680 - Flags: review?(ttaubert)
Comment on attachment 8448679 [details] [diff] [review]
Part 1, v1: code changes

Need to fix middleMousePasting and its test, too.
Attachment #8448679 - Flags: review?(dao)
Attachment #8448680 - Flags: review?(ttaubert)
Attached patch Part 1, v1.1: code changes (obsolete) — Splinter Review
I factored out the actual paste munging so that we can use it from browser.js - not sure if browser.js or tabbrowser or somewhere else is the best place, but I went wiht browser.js for now.
Attachment #8448708 - Flags: review?(dao)
Attachment #8448679 - Attachment is obsolete: true
Adjusted the failing test from the try push.
Attachment #8448712 - Flags: review?(ttaubert)
Attachment #8448680 - Attachment is obsolete: true
And here's a test for this functionality.
Attachment #8448726 - Flags: review?(dao)
Comment on attachment 8448708 [details] [diff] [review]
Part 1, v1.1: code changes

>+  // Then remove any unsafe protocols:
>+  clipboard = stripUnsafeProtocolsFromPaste(clipboard);

redundant comment, the function name says the same thing

>+function stripUnsafeProtocolsFromPaste(pasteData) {

how about "stripUnsafeProtocolForPasting", "...OnPaste" or "...FromCopiedString"?

>+  // Don't allow pasting in full URIs which inherit the security context.
>+  const kSecCtxFlag = Ci.nsIProtocolHandler.URI_INHERITS_SECURITY_CONTEXT;

just call this const URI_INHERITS_SECURITY_CONTEXT for a less cryptic name?

>+  let pastedURL;

pastedURI

>+  do {
>+    if (pastedURL) {
>+      pasteData = pastedURL.path.trim();
>+    } else {
>+      pasteData = pasteData.trim();

this line can move outside of the loop, or outside of the else branch, making the other trim() redundant

>+    }
>+    try {
>+      pastedURL = makeURI(pasteData);
>+    } catch (ex) {
>+      break;
>+    }
>+  } while (pastedURL && Services.netutil.URIChainHasFlags(pastedURL, kSecCtxFlag));

pastedURL should never be null here, so you can get rid of that check

>+              if (oldStart.trim()) {
>+                return;
>+              }

this is probably worth explaining

>+                // Creating a custom paste event with a modified value also
>+                // doesn't work.

no need to document this

>+                this.inputField.value = oldStart + pasteData + oldEnd;
>+                // Fix up cursor/selection:
>+                let newCursorPos = oldStart.length + pastedURL.path.length;

so close, but r-... pastedURL isn't declared here. I think pasteData.length is what you want? I see no reason to go back to nsIURI here.
Attachment #8448708 - Flags: review?(dao) → review-
D'oh about pastedURL. This should have your nits fixed.
Attachment #8448737 - Flags: review?(dao)
Attachment #8448708 - Attachment is obsolete: true
Cancelled the previous try push, new one at: https://tbpl.mozilla.org/?tree=Try&rev=4c62ac986e00
Attachment #8448737 - Flags: review?(dao) → review+
Attachment #8448712 - Flags: review?(ttaubert) → review+
Comment on attachment 8448726 [details] [diff] [review]
Part 3, v1: new test for the paste functionality

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

::: browser/base/content/test/general/browser_removeUnsafeProtocolsFromURLBarPaste.js
@@ +18,5 @@
> +
> +let clipboardHelper = Cc["@mozilla.org/widget/clipboardhelper;1"].getService(Ci.nsIClipboardHelper);
> +
> +function paste(input) {
> +  clipboardHelper.copyString(input);

Should be using waitForClipboard, like browser_middleMouse_noJSPaste.js does.
Attachment #8448726 - Flags: review?(dao) → review-
Attachment #8449342 - Flags: review?(bmcbride)
Attachment #8448726 - Attachment is obsolete: true
Attachment #8449342 - Flags: review?(bmcbride) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/474e76c20c55
remote:   https://hg.mozilla.org/integration/fx-team/rev/e6e23faf705f
remote:   https://hg.mozilla.org/integration/fx-team/rev/740ef090bec0
Whiteboard: [parity-Chrome][parity-IE] → [parity-Chrome][parity-IE][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/474e76c20c55
https://hg.mozilla.org/mozilla-central/rev/e6e23faf705f
https://hg.mozilla.org/mozilla-central/rev/740ef090bec0
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [parity-Chrome][parity-IE][fixed-in-fx-team] → [parity-Chrome][parity-IE]
Target Milestone: --- → Firefox 33
Hi Florin, can a QA contact be assigned for verification of this bug.
Flags: needinfo?(florin.mezei)
Flags: needinfo?(florin.mezei)
QA Contact: alexandra.lucinet
I've verified this on Win 7 x64, Ubuntu 13.04 x64, Mac OS X 10.9.3 using the latest Nightly build (BuildID: 20140703030200).

I've tested multiple cases, covering: stripping empty spaces, pasting "javascript:" as part of a string, pasting when characters/<space> have been written, cases from automatic tests, middle click paste (Auto Copy add-on), manual typing, the two cases from bug 1018583 comment 0 (OK now) and bug 1018583 comment 2 (Firefox still hangs). 
I've also compared the behavior with that on IE and Chrome.

The fix seems to be ok. For a confirmation please see the full results at https://docs.google.com/spreadsheets/d/1Mjc2kTJHqN2DeVjMLKwTzaNleVYes0Wa1vgcIfIyr7E/edit#gid=0, and let me know if I missed something or if this can be marked as VERIFIED. Note that there are a few cases where Firefox behaves like IE, and others where it behaves like Chrome. 

Gijs, can you have a look and confirm (or have someone else look at the results)?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #39)
> I've verified this on Win 7 x64, Ubuntu 13.04 x64, Mac OS X 10.9.3 using the
> latest Nightly build (BuildID: 20140703030200).
> 
> I've tested multiple cases, covering: stripping empty spaces, pasting
> "javascript:" as part of a string, pasting when characters/<space> have been
> written, cases from automatic tests, middle click paste (Auto Copy add-on),
> manual typing, the two cases from bug 1018583 comment 0 (OK now) and bug
> 1018583 comment 2 (Firefox still hangs). 
> I've also compared the behavior with that on IE and Chrome.
> 
> The fix seems to be ok. For a confirmation please see the full results at
> https://docs.google.com/spreadsheets/d/
> 1Mjc2kTJHqN2DeVjMLKwTzaNleVYes0Wa1vgcIfIyr7E/edit#gid=0, and let me know if
> I missed something or if this can be marked as VERIFIED. Note that there are
> a few cases where Firefox behaves like IE, and others where it behaves like
> Chrome. 
> 
> Gijs, can you have a look and confirm (or have someone else look at the
> results)?

This looks good to me, so I'm marking verified. Two notes:

On mac, on Chrome ( Version 35.0.1916.153 ), if I go to www.google.com, and then type and run:

javascript:alert(document.querySelectorAll("a").length)

I see an alert as expected (mine says 44, I wouldn't be surprised if yours says something else because you get a slightly different google homepage for whatever reason). In other words, I was surprised by E17 in your spreadsheet, which seems to imply Chrome doesn't run the JS, while in my testing it does do that.

Finally, I believe that the cases from bug 1018583 need the patches from that bug to land in order for them to be fixed - this was only about the URL bar.
Status: RESOLVED → VERIFIED
Flags: needinfo?(gijskruitbosch+bugs)
QA Whiteboard: [qa+] → [qa!]
Depends on: 1034077
Depends on: 1034845
Is there a pref that can be used to turn this off?  Or some other way to support the pretty common for standards work use case of opening a new tab and pasting a data: URI that got sent to the standards mailing list?
Flags: needinfo?(gijskruitbosch+bugs)
In particular, maybe we should keep doing the "don't inherit" thing for data: (where it makes way more sense than for javascript: and doesn't involve weird sandboxing stuff)?
(In reply to Boris Zbarsky [:bz] from comment #41)
> Is there a pref that can be used to turn this off?  Or some other way to
> support the pretty common for standards work use case of opening a new tab
> and pasting a data: URI that got sent to the standards mailing list?

Not really, no.

(In reply to Boris Zbarsky [:bz] from comment #42)
> In particular, maybe we should keep doing the "don't inherit" thing for
> data: (where it makes way more sense than for javascript: and doesn't
> involve weird sandboxing stuff)?

This will essentially involve reverting much of this bug. It would have been good if this had been brought up earlier... in any case, we could do that, I think - but please file a new bug.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #43)
> but please file a new bug.

Already filed - bug 1034845.
> It would have been good if this had been brought up earlier...

Well, the bug summary and comment 0 said nothing about data:.....
Depends on: CVE-2017-5458
Depends on: 1506100
You need to log in before you can comment on or make changes to this bug.