Consider blocking top level window data: URIs

RESOLVED FIXED in Firefox 56

Status

()

P2
normal
RESOLVED FIXED
2 years ago
11 months ago

People

(Reporter: jwatt, Assigned: ckerschb)

Tracking

(Depends on: 1 bug, {site-compat})

Trunk
mozilla56
site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 affected, firefox56 fixed)

Details

(Whiteboard: [domsecurity-active] spec change, URL)

Attachments

(5 attachments, 7 obsolete attachments)

11.77 KB, image/png
Details
271 bytes, text/html
Details
14.00 KB, patch
smaug
: review+
Details | Diff | Splinter Review
6.33 KB, patch
smaug
: review+
Details | Diff | Splinter Review
1.83 KB, patch
smaug
: review+
Details | Diff | Splinter Review
(Reporter)

Description

2 years ago
We should think about what we can do to block the sort of phishing attack described here:

https://www.wordfence.com/blog/2017/01/gmail-phishing-data-uri/

The use of a data: URI is interesting for three reasons I can think of.

 1) It allows the string "accounts.google.com" to appear reasonably
    far to the left of the address bar where the user who is looking
    for it is likely to see it. Also, since no attacker server is
    involved in loading the page, there is NO black highlight of an
    attack server's domain which the user's eye might be drawn to.

 2) Whether or not the new window is considered a secure context
    depends on the opener. In this case the opener, mail.google.com,
    is secure so the new window will be to. As a result password
    fields will not show a warning when the user types into them.

 3) Less importantly, the barrier to setting up such an attach is
    reduced since the attacker doesn't need to set up an attack
    server with a valid certificate (to avoid password field
    warnings). (Not much of a hurdle though.)

I don't know if we have any telemetry on how common [active content] top level window data: URIs are, but ideally I think we would disable them behind a pref which devs can turn on. The fact that IE/Edge don't support data: URIs gives me hope that we might be able to do that.

Failing that, maybe we could highlight the "data:" or "data:text/html" part is blue (as distinct from green or red) to draw the users attention to the fact that something is different.

Or maybe we could have a confirmation page alerting the user and requiring confirmation to proceed to load the page?

Or something...

Removing support for top level window data: URIs would also be nice because it would eliminate the need for end users to know anything about data: URIs.

Comment 1

2 years ago
Not supporting them top-level seems fine. It would be nice to have an option to enable them for development purposes, but I can understand if we rather not do that.
How about this?
  javascript:https://accounts.google.com/ServiceLogin?service=mail    %0a(whatever malicious code)
"https:" is valid as a label, and "//" is valid as a comment, so this is a valid JavaScript code. All browsers support top-level javascript: browsing contexts.

Comment 3

2 years ago
Unless I'm missing something, navigating to a javascript URL just executes script, it doesn't change the address bar.
  javascript:https://accounts.google.com/ServiceLogin?service=mail%0A"abc";

This URL changes the address bar with Firefox and IE11. (Chrome reverted the URL bar)
I'm going to reduce your attacks to 1) and 2). 2) is a problem with principal inheritance, which we're looking into breaking elsewhere. I think freddyb was going to add some telemetry for that?

1) could be addressed in lots of ways. We could canonicalize data URLs to base64, for instance. We canonicalize http urls for display, so why not? (devs would probably hate this -- makes tweaking hard.)

For purposes of the password warning, we could special-case data: urls to ignore the principal and give the "unsafe" warning anyway.
Priority: -- → P3
Whiteboard: [domsecurity-backlog2] spec change

Updated

2 years ago
Flags: needinfo?(tanvi)
We could add UI to the url bar / control center if we don't want to block entirely.  Keeping my needinfo and will go through this in more detail in a couple weeks.
If we block data URIs we also need to block javascript URIs, as an attacker could just use something like
> <a target=_blank href="javascript:document.write(PHISHING_CONTENT_HERE);">lul</a>

That means we need a carve out for bookmark clicks (because of JS bookmarklets) and possibly URLs coming directly into the awesome bar.
Well and blob:, while we're at it?
(Reporter)

Comment 9

2 years ago
(In reply to Masatoshi Kimura [:emk] from comment #2)
> How about this?
>   javascript:https://accounts.google.com/ServiceLogin?service=mail   
> %0a(whatever malicious code)
> "https:" is valid as a label, and "//" is valid as a comment, so this is a
> valid JavaScript code. All browsers support top-level javascript: browsing
> contexts.

When opening a new window the spec says to use the URI "about:blank" I believe. Anyway, if you can get firefox to behave questionably can you open a new bug for that issue please? (Maybe clone this bug so the same people get CC'ed.)
(Reporter)

Comment 10

2 years ago
(In reply to Frederik Braun [:freddyb] from comment #7)
> If we block data URIs we also need to block javascript URIs, as an attacker
> could just use something like
> > <a target=_blank href="javascript:document.write(PHISHING_CONTENT_HERE);">lul</a>

I don't think the issues with data: URIs and javascript:/blob: URIs are equivalent. A central part of my concern with the issue this bug covers is that top-level data: URI support allows bad actors to initiate phishing attacks from what are otherwise locked down environments. In other words sites which are not allowing third party javascript: URLs or Blobs & blob: URLs for obvious reasons.

It should be fairly obvious to any website developer that javascript: URLs should be filtered out from any third party content embedded into their site. However, it is far less obvious that they should be filtering data: URLs. I'd bet most sites filter javascript: and most don't filter data:. Hence why I think we should take action regarding data: URLs.

Also note that since javascript: URL support is ubiquitous we pretty much can't block that from top level navigation without breaking an unacceptably large number of sites. As noted above, the lack of data: URI support in IE/Edge means that this is far less likely to be the case for data: URIs.
(Reporter)

Comment 11

2 years ago
(In reply to Tanvi Vyas - not reading bugmail, PM me [:tanvi] from comment #6)
> We could add UI to the url bar / control center if we don't want to block
> entirely.

I think we should block entirely (with a pref devs can use to enable for their convenience) unless or until we discover that just won't work.
(Reporter)

Comment 12

2 years ago
(In reply to Daniel Veditz [:dveditz] from comment #5)
> 2) is a problem with
> principal inheritance, which we're looking into breaking elsewhere. I think
> freddyb was going to add some telemetry for that?

Yeah, I have concerns about blocking the principal inheritance. See bug 1328860 comment 1.

> 1) could be addressed in lots of ways. We could canonicalize data URLs to
> base64, for instance. We canonicalize http urls for display, so why not?
> (devs would probably hate this -- makes tweaking hard.)

I use data: URIs a lot, and I would find that really annoying. I suspect most devs like me would prefer a pref to enable the current behavior.

> For purposes of the password warning, we could special-case data: urls to
> ignore the principal and give the "unsafe" warning anyway.

We could, true. But rather than adding complexity by having exceptions and different behaviors it seems better to block, unless someone comes up with some credible reasons for why that is a bad idea.
In reply to Jonathan Watt [:jwatt] from comment #11)
> (In reply to Tanvi Vyas - not reading bugmail, PM me [:tanvi] from comment
> #6)
> > We could add UI to the url bar / control center if we don't want to block
> > entirely.
> 
> I think we should block entirely (with a pref devs can use to enable for
> their convenience) unless or until we discover that just won't work.

Let's have a look at our test-suite. I suppose there shouldn't be that many tests which we would have to update by adding a pref-flip (to allow data: URIs in the top level doc) for the test run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=75fe3572543d42bed1e5ca1d88eca8d30691b1c5

In general, I suppose we could block data: URIs within the top level doc, but should we probably allow data: URIs if typed explicitly into the address bar?
(In reply to Jonathan Watt [:jwatt] from comment #10)
> I don't think the issues with data: URIs and javascript:/blob: URIs are
> equivalent. A central part of my concern with the issue this bug covers is
> that top-level data: URI support allows bad actors to initiate phishing
> attacks from what are otherwise locked down environments. In other words
> sites which are not allowing third party javascript: URLs or Blobs & blob:
> URLs for obvious reasons.

You started this thread pointing to a phishing example. If we want to solve this problem, we should solve for all URI schemes that allow this sort of phishing attack, no?
(In reply to Frederik Braun [:freddyb] from comment #14)
> You started this thread pointing to a phishing example. If we want to solve
> this problem, we should solve for all URI schemes that allow this sort of
> phishing attack, no?

Yes, and the work to cover these other schemes could be done in follow-up bugs.

This particular one (data: URIs) is actively being used to attack users and is already blocked by Edge and (very soon) Chrome. We should close that attack vector quickly too.
(In reply to François Marier [:francois] from comment #15)
> the work to cover these other schemes could be done in follow-up bugs.

fair enough, just wanted to emphasize this isn't left out completely.
(Assignee)

Updated

2 years ago
Depends on: 1357386
Duplicate of this bug: 1360485
(Assignee)

Updated

2 years ago
Depends on: 1377567
I created a test page for this bug: https://mozilla.github.io/domsecurity-test/1331351.html

and noticed that Chrome 61 blocks these navigations silently without any UI except for the following console message:

> Not allowed to navigate top frame to data URL: data:text/html,<h1>data: URI page</h1><p>This page is contained entirely within the URL.</p>

while Chrome 59 only shows "Not secure" in the URL bar. Both Chrome versions allow users to type in the data: URI manually in the address bar.

On Edge, neither typing the URL directly nor navigating to it work.
(In reply to François Marier [:francois] from comment #18)
> Created attachment 8883107 [details]
> Navigating to a data: URI on Edge
> 
> I created a test page for this bug:
> https://mozilla.github.io/domsecurity-test/1331351.html
> 
> and noticed that Chrome 61 blocks these navigations silently without any UI
> except for the following console message:
> 
> > Not allowed to navigate top frame to data URL: data:text/html,<h1>data: URI page</h1><p>This page is contained entirely within the URL.</p>
> 
> while Chrome 59 only shows "Not secure" in the URL bar. Both Chrome versions
> allow users to type in the data: URI manually in the address bar.
> 
> On Edge, neither typing the URL directly nor navigating to it work.

Thanks for the verification Francois. I think once we have refined our telemtry within Bug 1377567, we will go down a very similar route:
- Allow data: URIs to appear in toplevel window if typed in by the user.
- Block navigations to data: URIs in toplevel window
(Assignee)

Updated

2 years ago
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Created another local testpage for that.
Hey Francois, before moving forward I wanted to run this by you to make sure we agree on the approach. Within this patch we:
a) block toplevel navigations to a data: URI,
b) allow toplevel data: URIs if typed in by the user
c) we can't reliably distinguish between loads coming from outside (e.g. thunderbird) hence we are allowing those as well for now.

I tested various scenarios:
1) Clicking on a data: URI link (e.g. using the attached testcase from the previous comment). At the moement we silently ignore the click (it feels like nothing is happening) besides the browser console message we log:
> Navigation to toplevel data: URI not allowed (Blocked loading of: “data:text/html,<body>toplevel%20data:%20URI%20navi...”)

2) Using right-click-open-link-in-new-tab, a new tab opens with the data: URI displayed in the URL bar, but the actual data: URI is blocked within the browser; browser message is logged

3) Using ctrl-click, completely the same as (2)

Do we agree on all that or do we rather want some more sophisticated UI around that?


======================
TESTING:
I wrote a preliminary browser chrome test for this, but the problem is that all of our link click utils load using a systemprincipal and not, like in the regular user link click case, using a codebaseprincipal. Hence testing this is difficult. I tried using:

*) let myLink = browser.contentDocument.getElementById("testlink");
   myLink.click();

*) var ev = content.document.createEvent('MouseEvents');
   ev.initMouseEvent("click", true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
   let myLink = content.document.getElementById("testlink");
   myLink.dispatchEvent(ev);

*) BrowserTestUtils.synthesizeMouseAtCenter(myLink, {button: 1}, browser);

In all of these cases we load using the systemPrincipal. If you have any suggestions on how we could test this, please let me know.
Flags: needinfo?(tanvi)
Attachment #8885260 - Flags: feedback?(francois)
For the sake of completeness, Dan suggested we could potentially rely on LOAD_FLAGS_FROM_EXTERNAL [1]. I would be willing to update the patch to take that into account and also block data: URI loads that load using a systemPrincipal but having the LOAD_FLAGS_FROM_EXTERNAL flag set.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1377567#c3
(Assignee)

Updated

2 years ago
Priority: P3 → P2
Whiteboard: [domsecurity-backlog2] spec change → [domsecurity-active] spec change
Comment on attachment 8885260 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris.patch

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

> Do we agree on all that or do we rather want some more sophisticated UI
> around that?

Given that other browsers don't have any UI for this, I think that the console message is good enough. Your approach looks fine to me.

Also: datareview+ for the changes to Histograms.json
Attachment #8885260 - Flags: feedback?(francois) → feedback+
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #22)
> For the sake of completeness, Dan suggested we could potentially rely on
> LOAD_FLAGS_FROM_EXTERNAL [1]. I would be willing to update the patch to take
> that into account and also block data: URI loads that load using a
> systemPrincipal but having the LOAD_FLAGS_FROM_EXTERNAL flag set.

If it works, that sounds very worthwhile too.
Hey Smaug, we would like to block toplevel data: URI navigations. Initially I wanted to use the principal to determine the navigation (see comment 21), but I think using the referrer is what we should use in the end to determine whether a link got navigated or typed into the URL-Bar. Please note that we don't want to block data: URIs typed into the URL-Bar by the user, but we don't want to block data: URIs that are navigated and also come from an external application (e.g. ThunderBird). Happy to discuss more on IRC or also in person if needed.
Attachment #8885260 - Attachment is obsolete: true
Attachment #8885725 - Flags: review?(bugs)
Do we have aReferrerURI around even when noreferrer is used?
(In reply to Olli Pettay [:smaug] from comment #27)
> Do we have aReferrerURI around even when noreferrer is used?

As far as I know we are just setting the flag |flags |= INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER | and then just suppress the referrer, but the actual referrer-uri is available at that point in the nsDocShell. Looking at the initial patch for noreferrer [1] confirms that. We can obviously can include a testcase for that.

[1] https://hg.mozilla.org/mozilla-central/rev/e4d2928e0713
Have you tested cases like domain A opening window.open(); (which just creates initial about:blank) and then using that a new window is loaded. Is the referrer set the way the patch expects?
Flags: needinfo?(ckerschb)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #25)
> I wanted to use the principal to determine the navigation (see comment 21),
> but I think using the referrer is what we should use in the end to determine
> whether a link got navigated or typed into the URL-Bar.

One of the difficulties in using the referrer (and the reason why I still haven't finished the patch for bug 446344) is that when a user disables the referrer (or spoofs it), we don't keep an "internal" version of the real one for our own uses. So until we can fix that and add a referrer that survives all of the various ways to truncate or disable it, it may be hard to rely on it.
Comment on attachment 8885725 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris.patch

ok, that is clearly a hint something else is needed.
Attachment #8885725 - Flags: review?(bugs)
Comment on attachment 8885726 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris_tests.patch

Need to test also window.open.
Attachment #8885726 - Flags: feedback?(bugs) → feedback+
(In reply to François Marier [:francois] from comment #30)
> One of the difficulties in using the referrer (and the reason why I still
> haven't finished the patch for bug 446344) is that when a user disables the
> referrer (or spoofs it), we don't keep an "internal" version of the real one
> for our own uses. So until we can fix that and add a referrer that survives
> all of the various ways to truncate or disable it, it may be hard to rely on
> it.

The main reason I was relying on the referrer instead of the triggeringPrincipal was testing, see comment 21 (bottom). If we can figure out testing then we should rely on the principal. Smaug, what do you think about using the triggeringPrincipal as the distinguishing factor?
Flags: needinfo?(bugs)
(In reply to François Marier [:francois] from comment #30)
> One of the difficulties in using the referrer (and the reason why I still
> haven't finished the patch for bug 446344) is that when a user disables the
> referrer (or spoofs it), we don't keep an "internal" version of the real one
> for our own uses. So until we can fix that and add a referrer that survives
> all of the various ways to truncate or disable it, it may be hard to rely on
> it.

Thinking about that further, I doubt that actually, because we are relying on the referrer to create a principal, which would mean we are facing serious problems if what you are saying is true. Just to make sure, how do I disable the referrer? I will test that right away.
Flags: needinfo?(francois)
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #34)
> Thinking about that further, I doubt that actually, because we are relying
> on the referrer to create a principal, which would mean we are facing
> serious problems if what you are saying is true. Just to make sure, how do I
> disable the referrer? I will test that right away.

It seems that there is a variable aSendReferrer which causes us not to send out the actual referrer in the very last minute, but the value aReferrer is still available and accurate. I will add testcases for that to make sure we don't regress that ever.

[1] https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#11329
Why browser chrome test. Shouldn't it be mochitest running non-chrome JS.
triggeringPrincipal does conceptually sound better.
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #34)
> Thinking about that further, I doubt that actually, because we are relying
> on the referrer to create a principal, which would mean we are facing
> serious problems if what you are saying is true. Just to make sure, how do I
> disable the referrer? I will test that right away.

A few ways to remove it entirely:

network.http.sendRefererHeader = 0
network.http.referer.spoofSource = true
network.http.referer.userControlPolicy = 0

and more ways to remove it cross-origin:

network.http.referer.userControlPolicy = 1
network.http.referer.XOriginPolicy = 2
Flags: needinfo?(francois)
(In reply to François Marier [:francois] from comment #37)
> A few ways to remove it entirely:

Thanks Francois.
Smaug, I went back to using the principal approach to determine whether a link was navigated or not. I know use a mochitest (next patch) instead of a browser test. We still need a couple more tests in the end, but at least that testing approach works now as expected - thanks for the hint.
Attachment #8885725 - Attachment is obsolete: true
Attachment #8885726 - Attachment is obsolete: true
Flags: needinfo?(ckerschb)
Attachment #8885884 - Flags: review?(bugs)
Also clearing your needinfo, because you already answered my question within comment 36.
Flags: needinfo?(bugs)
Comment on attachment 8885884 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris.patch

It seems that out testsuite relies on that behavior quite often. Dunno how hard it is to update all of those failing tests, but I need to take a closer look:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=faf996f2f83d68acbd8966b70e3e2ff0b5baca9c&selectedJob=113791122
Attachment #8885884 - Flags: review?(bugs)
(Assignee)

Updated

2 years ago
Attachment #8885885 - Flags: feedback?(bugs)
I took a closer look at the failing tests from comment 42 and it seems that lots of our tests are actually navigating to a toplevel data: URI - exactly what we want to prevent within this patch. Some of those cases are:

a) dom/base/test/test_data_uri.html
   var win = window.open("data:text,...

b) layout/base/tests/test_bug607529.html
  var w = window.open("file_bug607529.html");
  w.location = "data:text/html,<script...

c) browser/base/content/test/general/browser_bug575561.js
  <a href="data:text/html,<!DOCTYPE ...
  link.click();

Overall we would have to update 102 failing tests, which makes this bug a bigger task than we anticipated. On the bright side, our approach seems to work just fine :-)

FAILING TESTS:
==============
browser/  18
devtools/ 16
docshell/  7
dom/      35
editor/    5
layout/   12
toolkit/   9
============
TOTAL    102
I think what we can do is the following: Land the patch behind a pref and then start updating tests till we have converted all of them and finally switch on the pref when ready.
(Assignee)

Updated

2 years ago
Blocks: 1380959
Hey Smaug, as discussed on IRC, let's land that change behind a pref. I filed Bug 1380959 to switch the pref to true once we have updated all the failing tests. Does that sounds like a good path forward?
Attachment #8885884 - Attachment is obsolete: true
Attachment #8885885 - Attachment is obsolete: true
Attachment #8886557 - Flags: review?(bugs)
For the test, I explicitly flip the pref. Happy to incorporate more tests if needed.
Attachment #8886558 - Flags: review?(bugs)
Comment on attachment 8886558 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris_mochitests.patch

s/naviagtions/navigations/

Need to test also window.open("data:...")

We need also test for window.location.href = "data:text/html,..."
when window is a top level page.

And <a href="data:..." target="_blank">
Attachment #8886558 - Flags: review?(bugs) → review-
Comment on attachment 8886557 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris.patch

>+  if (nsIOService::BlockToplevelDataUriNavigations() &&
>+      aContentPolicyType == nsIContentPolicy::TYPE_DOCUMENT) {
First check aContentPolicyType == nsIContentPolicy::TYPE_DOCUMENT and then
BlockToplevelDataUriNavigations 

>+    bool isDataURI =
>+      (NS_SUCCEEDED(aURI->SchemeIs("data", &isDataURI)) && isDataURI);
>+    // Let's block all toplevel document navigations to a data: URI.
>+    // In all cases where the toplevel document is navigated to a
>+    // data: URI the triggeringPrincipal is a codeBasePrincipal.
>+    // In other cases, e.g. typing a data: URL into the URL-Bar,
>+    // the triggeringPrincipal is a SystemPrincipal - we don't
>+    // want to block those loads. Only exception, loads coming
>+    // from an external applicaton (e.g. ThunderBird) don't load
s/applicaton/application/
it is Thunderbird, not ThunderBird

>+    // using a codeBasePrincipal, but we want to block those loads.
>+    if (isDataURI && (aLoadFromExternal || 
>+        aTriggeringPrincipal->GetIsCodebasePrincipal())) {
So if triggering principal is a null principal, we don't return security error
(and not external load)

Looks like nullprincipal case is something to test too.
Attachment #8886557 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #48)
> So if triggering principal is a null principal, we don't return security
> error

Smaug, I looked at this a little closer again. In fact we should block toplevel navigations not only if triggered by a codebasePrincipal, but also from a NullPrincipal (e.g. in the case when a data: URI iframe loads a data: URI in target=_blank case. So I modified that part slightly to only not return a security error if the system causes a toplevel data: URI navigation:

Further I added the following mochitests:
* toplevel navigation to data: URI using click() [uses codebaseprincipal]
* data: iframe performs win.open(..., "_blank") [uses nullPrincipal]
* window.location.href [uses codebaseprincipal]
* window.open(data:) [uses codebaseprincipal]

And also the following browser tests:
* open new tab [uses systemprincipal]

Unfortunately, we have to use flaky timeouts to test that a toplevel navigation actually did *not* succeed. I guess that's suboptimal, but I also don't know of any better alternative.
Attachment #8886557 - Attachment is obsolete: true
Attachment #8886558 - Attachment is obsolete: true
Attachment #8888269 - Flags: review?(bugs)
Comment on attachment 8888269 [details] [diff] [review]
bug_1331351_block_toplevel_data_uris.patch

>+      return NS_ERROR_DOM_SECURITY_ERR;
If my testing is right, Chrome doesn't seem to throw any exception.
Could you test too, and use NS_OK ?

>+BlockTopLevelDataURINavigation=Navigation to toplevel data: URI not allowed (Blocked loading of: “%1$S”)
Something odd at the end of the line?

>+// TODO: Bug 1380959: Block toplevel data: URI navigations
>+// If tue, all toplevel data: URI navigations will be blocked.
true
Attachment #8888269 - Flags: review?(bugs) → review+
Attachment #8888270 - Flags: review?(bugs) → review+
Attachment #8888272 - Flags: review?(bugs) → review+

Comment 53

2 years ago
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a721ec6c0170
Block toplevel window data: URI navigations. r=smaug,francois
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cd4b2d56258
Test block toplevel window data: URI navigations. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/e07bc6167928
Test allow toplevel window data: URI navigations from system. r=smaug
(Assignee)

Updated

2 years ago
Depends on: 1384054
(In reply to Pulsebot from comment #54)
> Pushed by mozilla@christophkerschbaumer.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a8bf894267b6
> Disable mochitest on android. r=me

Please note that the whole feature lives behind a pref for now. I filed Bug 1384054 to re-enable the test on android.

Comment 56

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a721ec6c0170
https://hg.mozilla.org/mozilla-central/rev/9cd4b2d56258
https://hg.mozilla.org/mozilla-central/rev/e07bc6167928
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Updated

2 years ago
Depends on: 1395948
Depends on: 1394554

Comment 58

a year ago
Sure wish you would let the user decide if they want to load a data URL similar to the way they decide if they want to load a pop-up.

You rely on this functionality internally 102 times. It must be useful.

I saw absolutely nowhere in this discussion where you decide if this would have an effect on users besides yourselves. Which should have been a flag since you use it quite regularly.

I hardly think that "well, Micro$oft does this already" is a good way to develop a browser. In fact I think it was Micro$oft's development practices that led to the rise of Firefox.

Sincerely,

A web based app developer who uses data URL's responsibly and with good reasons.
(In reply to Buck Manhands from comment #58)
> Sure wish you would let the user decide if they want to load a data URL
> similar to the way they decide if they want to load a pop-up.
The user can always copy/paste the data URL into the URL bar which is still allowed.

> You rely on this functionality internally 102 times. It must be useful.
The only reason we used this internally was convenience, because it allowed us to write tests faster. We updated all of those tests.

> I saw absolutely nowhere in this discussion where you decide if this would
> have an effect on users besides yourselves. Which should have been a flag
> since you use it quite regularly.
Mostly we use bugs for technical discussion. The broader usability discussion happens on dev-platform, see:
https://www.mail-archive.com/dev-platform@lists.mozilla.org/msg23527.html

> A web based app developer who uses data URL's responsibly and with good
> reasons.
That is indeed the problem, it's hard to distinguish between the good and the bad folks out there.

Comment 60

a year ago
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #59)

> The user can always copy/paste the data URL into the URL bar which is still
> allowed.

lol, yeah, that's how most people access a link anyways. Great solution :| - my client should love that new workflow.

> The only reason we used this internally was convenience, because it allowed
> us to write tests faster. We updated all of those tests.

convenience for the dev and the user seems reason enough to seek to keep a perfectly valid way to use web technology.

> Mostly we use bugs for technical discussion. The broader usability
> discussion happens on dev-platform, see:
> https://www.mail-archive.com/dev-platform@lists.mozilla.org/msg23527.html

thanks. Not much discussion, more a notice in this case.

> That is indeed the problem, it's hard to distinguish between the good and
> the bad folks out there.

Right, which is why you create code in a way that distinguishes good CODE and bad CODE and let the end user decide. Banning something never works. There is already a simple iFrame workaround to this issue that leaves the end user just as screwed by phishing attempts as before and for me works poorly in this case because it makes the download button in the PDF viewer stop functioning. Phishermen win, I lose. And you all no longer have a convenient way of doing your testing.
Duplicate of this bug: 1416092
You need to log in before you can comment on or make changes to this bug.