Closed Bug 1380959 Opened 2 years ago Closed 2 years ago

Block toplevel data: URI navigations in Nightly and early Beta

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ckerschb, Assigned: ckerschb)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [domsecurity-active])

Attachments

(1 file, 1 obsolete file)

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
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Depends on: 1331351
Priority: -- → P2
Whiteboard: [domsecurity-active]
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
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...
(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.
> 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.
See Also: → 1258542
Depends on: 1396749
(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
Depends on: 1396798
Depends on: 1397651
Depends on: 1397652
Depends on: 1397653
Depends on: 1397655
Depends on: 1397656
Depends on: 1397657
Depends on: 1398574
Depends on: 1398691
Depends on: 1398692
Depends on: 1399074
Depends on: 1399468
Depends on: 1399848
Summary: Block toplevel data: URI navigations → Bug 1380959: Block oplevel data: URI navigations in Nightly and early Beta
As agreed, here is a patch that will only enable the change in behavior for Nightly and early Beta.
Attachment #8908575 - Attachment is obsolete: true
Flags: needinfo?(bzbarsky)
Comment on attachment 8908679 [details] [diff] [review]
bug_1380959_block_top_level_data_uri_navigations_in_nightly.patch

r=me
Flags: needinfo?(bzbarsky)
Attachment #8908679 - Flags: review+
Depends on: 1400347
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4360d673edef
Block oplevel data: URI navigations in Nightly and early Beta. r=bz
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
Flags: needinfo?(ckerschb)
Summary: Bug 1380959: Block oplevel data: URI navigations in Nightly and early Beta → Block toplevel data: URI navigations in Nightly and early Beta
Depends on: 1400473
(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.
Flags: needinfo?(ckerschb)
Priority: P2 → P1
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c1e11c61824
Block oplevel data: URI navigations in Nightly and early Beta. r=bz
https://hg.mozilla.org/mozilla-central/rev/2c1e11c61824
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Blocks: 1401895
(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!
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!
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?
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.
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.
Depends on: 1403814
Depends on: 1403870
Depends on: 1407891
See Also: → 1552715
You need to log in before you can comment on or make changes to this bug.