1380959 - Block toplevel data: URI navigations in Nightly and early Beta

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P1)

Description

[Christoph Kerschbaumer [:ckerschb]]

Within Bug 1331351 we are going to land the actual code for blocking toplevel data: URI navigations behind a pref. Since so many tests within our codebase, see [1] are relying on that behavior we keep this bug as a meta bug. Convert all the tests and then flip on the pref within this bug once all tests in our codebase are converted.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1331351#c43

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

It seems that we can already eliminate 40 of those tests by rewriting window.open("data... [1] to load those data: URIs from an external. file.


[1] https://dxr.mozilla.org/mozilla-central/search?q=window.open(%22data&redirect=false

Comment 2

[Boris Zbarsky [:bzbarsky]]

Is there a clear summary somewhere of the problems this is trying to solve?

Because it's not entirely clear why we're blocking all data: URIs, including ones we wouldn't handle internally, image types, PDF, etc.  For the phishing use cases, just blocking document types would seem like the right thing...

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] from comment #2)
> Because it's not entirely clear why we're blocking all data: URIs, including
> ones we wouldn't handle internally, image types, PDF, etc.  For the phishing
> use cases, just blocking document types would seem like the right thing...

We only want to block top-level data URI navigations which are mostly used for phishing. I don't see any actual usecase for those navigations (besides actual phishing attempts). We installed the pref |block_toplevel_data_uri_navigations| which only blocks toplevel data: URI loads if navigated - see Bug 1331351. Please note that we don't even want to block top-level data: URIs if typed in the address bar, because I think webdevs are using that.

Before we can flip that pref though, we have to update our testsuite because we are using toplevel data: URI navigations for testing purposes - mostly for convenience though - as far I as can tell from a quick DXR lookthrough.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> I don't see any actual usecase for those navigations (besides actual phishing attempts).

You should read the Chrome intent thread, where people cite usecases.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #4)
> > I don't see any actual usecase for those navigations (besides actual phishing attempts).
> 
> You should read the Chrome intent thread, where people cite usecases.

I started to generate a list of use cases - see here:
https://public.etherpad-mozilla.org/p/blockingdatanav

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1380959_block_top_level_data_uri_navigations.patch

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1380959_block_top_level_data_uri_navigations_in_nightly.patch

As agreed, here is a patch that will only enable the change in behavior for Nightly and early Beta.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8908679 [details] [diff] [review]
bug_1380959_block_top_level_data_uri_navigations_in_nightly.patch

r=me

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4360d673edef
Block oplevel data: URI navigations in Nightly and early Beta. r=bz

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out bug 1400347 and bug 1380959 for failing talos tpaint:

bug 1380959: https://hg.mozilla.org/integration/mozilla-inbound/rev/2d5650ab1aeca067c7b21584e73813761ffc4dca
bug 1400347: https://hg.mozilla.org/integration/mozilla-inbound/rev/1a6381a51810fed75d99cdf04b6735abfe78922a

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=906b8bdd2002b75be1ec731694d818b7aff02187&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=131354755&repo=mozilla-inbound

12:36:01     INFO -  TEST-INFO | started process 13826 (/builds/slave/test/build/application/firefox/firefox -profile /tmp/tmp1v003p/profile -tp file:/builds/slave/test/build/tests/talos/talos/tests/tpaint/tpaint.manifest.develop -tpchrome -tpmozafterpaint -tpnoisy -tpcycles 1 -tppagecycles 20)
12:36:02     INFO -  PID 13826 |
12:36:02     INFO -  PID 13826 | (/builds/slave/test/build/application/firefox/firefox:13875): Pango-WARNING **: error opening config file '/home/cltbld/.pangorc': Permission denied
12:36:02     INFO -  PID 13826 |
12:36:12     INFO -  PID 13826 | RSS: Main: 179146752
12:36:12     INFO -  PID 13826 |
12:41:01     INFO -  Timeout waiting for test completion; killing browser...
12:41:01     INFO -  Terminating psutil.Process(pid=13826, name='firefox')
12:41:01     INFO -  PID 13826 | ExceptionHandler::GenerateDump cloned child 13962
12:41:01     INFO -  PID 13826 | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
12:41:01     INFO -  PID 13826 | ExceptionHandler::WaitForContinueSignal waiting for continue signal...
12:41:01     INFO -  TEST-UNEXPECTED-ERROR | tpaint | timeout
12:41:01    ERROR -  Traceback (most recent call last):
12:41:01     INFO -    File "/builds/slave/test/build/tests/talos/talos/run_tests.py", line 281, in run_tests
12:41:01     INFO -      talos_results.add(mytest.runTest(browser_config, test))
12:41:01     INFO -    File "/builds/slave/test/build/tests/talos/talos/ttest.py", line 61, in runTest
12:41:01     INFO -      return self._runTest(browser_config, test_config, setup)
12:41:01     INFO -    File "/builds/slave/test/build/tests/talos/talos/ttest.py", line 209, in _runTest
12:41:01     INFO -      if counter_management else None),
12:41:01     INFO -    File "/builds/slave/test/build/tests/talos/talos/talos_process.py", line 130, in run_browser
12:41:01     INFO -      raise TalosError("timeout")
12:41:01     INFO -  TalosError: timeout

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #10)
> Backed out bug 1400347 and bug 1380959 for failing talos tpaint:

Sorry for the bustage. I filed Bug 1400473 to get that talos tpaint updated.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c1e11c61824
Block oplevel data: URI navigations in Nightly and early Beta. r=bz

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2c1e11c61824

Comment 14

[Kohei Yoshino]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2017/data-url-navigations-on-top-level-window-will-be-blocked/

Comment 15

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Kohei Yoshino [:kohei] from comment #14)
> Posted the site compatibility doc:
> https://www.fxsitecompat.com/en-CA/docs/2017/data-url-navigations-on-top-
> level-window-will-be-blocked/

Thanks, that is great!

Comment 16

[Chris Mills [:cmills]]

I've documented this by adding a note to our experimental features page at

https://developer.mozilla.org/en-US/Firefox/Experimental_features#Other

Let me know if this looks OK; thanks!

Comment 17

[zdnex]

Hello so I have found this bug when I was trying figure out why download suddenly stopped working on several pages... as you can see here.... https://bugzilla.mozilla.org/show_bug.cgi?id=1403641 Is directly connected?

Comment 18

[zdnex]

I am wondering... is this really good? I have just tried several commonly used data: URIs and it seems that several are broken with this change... Will this solve something? As if this is really planned change I am really leaving Firefox. New menu in 57 (broken), missing good old download button and now I got this really unwanted change which breaks several quite nice usage for data: URI just because it looks like only phishing sites are using it.

Comment 19

[Boris Zbarsky [:bzbarsky]]

If you have concrete in-the-wild uses of data: URIs that this breaks, other than the one already covered by bug 1403641), please file bugs with links to them and mark them blocking this bug and bug 1401895.