1558394 - To block automatic download in sandboxed iframe

Status: RESOLVED FIXED

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

Description

[Yao Xiao]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.80 Safari/537.36

Steps to reproduce:

Trigger an automatic download in sandboxed iframe.

Actual results:

Download is not blocked

Expected results:

Download should be blocked - preventing automatic download in sandboxed iframe should be the default as downloads can bring security vulnerabilities to the system.

I'm hoping to see implementer interest.

whatwg/html discussion: https://github.com/whatwg/html/issues/3236
PR: https://github.com/whatwg/html/pull/4293
WPT (already checked in): https://github.com/web-platform-tests/wpt/commit/245334dcc1695c3dbc4e1fcdbe849224234093fc

Comment 1

[Liviu Seplecan]

Hi @Yao Xiao, please provide a TC or complete data in order to test the issue - triggering an automatic download in sandbox iframe.
Additionally, looking at the description, I guess this is more an enhancement or something that could/need to be implemented.
I will set a component to the issue and if isn't the right one please fell free to change it. Further, someone from dev's team can confirm if is an issue or an enhancement.

Comment 2

[Yao Xiao]

You can test this issue in https://cr.kungfoo.net/yao/downloads/auto_download_in_sandbox.html, where the page has a sandboxed iframe with src= a downloadable/non-renderable file. The intended behavior is download does not happen; in Chrome 75, download still happens but you will see a DevTool message saying "[Deprecation] Download in sandbox without user activation is deprecated and will be removed in M76 ...".

Note that the detailed behavior is proposed in PR: https://github.com/whatwg/html/pull/4293. The test above does not intend to cover every scenario.

Also, there are already some Web Platform Tests checked in, but disabled, in FireFox:
e.g. https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/meta/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_anchor_download_block_downloads_without_user_activation.sub.tentative.html.ini

I'm not sure about the convention in FireFox whether to call this a bug/feature/enhancement/... The idea is I wish to see implementor interests from FireFox - it's a good practice to check in the HTML spec change if other major vendors can agree this change is good.

Comment 3

[Sebastian Streich [:sstreich]]

Attachment: Bug 1558394 - Block downloads in sandboxed iframes r=ckerschb

Comment 4

[Sebastian Streich [:sstreich]]

Attachment: Bug 1558394 - Pass the TriggeringSandboxFlags to nsILoadinfo r=ckerschb

Add triggering Sandbox flags to loadinfo

---

Pass triggering Flags into Loadinfo

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80a0ea17c9f8
Pass the TriggeringSandboxFlags to nsILoadinfo r=ckerschb,smaug,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/5889105bd089
Block downloads in sandboxed iframes r=ckerschb,smaug

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/23598 for changes under testing/web-platform/tests

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 2 changesets (bug 1558394) for nsDocShellLoadState related bustage

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunning%2Cpending%2Crunnable&fromchange=5889105bd08941784888442e638829a2561f900c&searchStr=build&tochange=c6808d59644cf49ca9e1f97815b616e18af33d71

Backout link: https://hg.mozilla.org/integration/autoland/rev/c6808d59644cf49ca9e1f97815b616e18af33d71

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=302255446&repo=autoland

[task 2020-05-14T12:13:15.078Z] 12:13:15     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/docshell/base'
[task 2020-05-14T12:13:15.082Z] 12:13:15     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_docshell_base0.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/docshell/base -I/builds/worker/workspace/obj-build/docshell/base -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/docshell/shistory -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/bindings -I/builds/worker/checkouts/gecko/js/xpconnect/src -I/builds/worker/checkouts/gecko/layout/base -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/checkouts/gecko/layout/style -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/netwerk/base -I/builds/worker/checkouts/gecko/netwerk/protocol/viewsource -I/builds/worker/checkouts/gecko/toolkit/components/browser -I/builds/worker/checkouts/gecko/toolkit/components/find -I/builds/worker/checkouts/gecko/tools/profiler -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -fexperimental-new-pass-manager  -MD -MP -MF .deps/Unified_cpp_docshell_base0.o.pp   Unified_cpp_docshell_base0.cpp
[task 2020-05-14T12:13:15.084Z] 12:13:15     INFO -  In file included from Unified_cpp_docshell_base0.cpp:119:
[task 2020-05-14T12:13:15.085Z] 12:13:15    ERROR -  /builds/worker/checkouts/gecko/docshell/base/nsDocShellLoadState.cpp:96:7: error: field 'mFileName' will be initialized after field 'mTriggeringSandboxFlags' [-Werror,-Wreorder]
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -        mFileName(VoidString()),
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -        ^
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -  In file included from Unified_cpp_docshell_base0.cpp:92:
[task 2020-05-14T12:13:15.085Z] 12:13:15  WARNING -  /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:7623:24: warning: nsIPrincipal->GetURI is depricated and will be removed soon. Please consider using the new helper functions of nsIPrincipal
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -          thisPrincipal->GetURI(getter_AddRefs(prinURI));
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -                         ^
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -  1 warning and 1 error generated.
[task 2020-05-14T12:13:15.085Z] 12:13:15     INFO -  /builds/worker/checkouts/gecko/config/rules.mk:750: recipe for target 'Unified_cpp_docshell_base0.o' failed
[task 2020-05-14T12:13:15.086Z] 12:13:15    ERROR -  make[4]: *** [Unified_cpp_docshell_base0.o] Error 1
[task 2020-05-14T12:13:15.086Z] 12:13:15     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/docshell/base'
[task 2020-05-14T12:13:15.086Z] 12:13:15     INFO -  /builds/worker/checkouts/gecko/config/recurse.mk:74: recipe for target 'docshell/base/target-objects' failed
[task 2020-05-14T12:13:15.086Z] 12:13:15    ERROR -  make[3]: *** [docshell/base/target-objects] Error 2
[task 2020-05-14T12:13:15.086Z] 12:13:15     INFO -  make[3]: *** Waiting for unfinished jobs....
[task 2020-05-14T12:13:15.089Z] 12:13:15     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/accessible/atk'

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 10

[Sebastian Streich [:sstreich]]

Sorry about that. Fixed the Bustage :)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e13ede3c68d4
Pass the TriggeringSandboxFlags to nsILoadinfo r=ckerschb,smaug,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/79046ff8143b
Block downloads in sandboxed iframes r=ckerschb,smaug

Comment 12

[Razvan Maries]

Backed out for perma failures on iframe_sandbox_anchor_download_block_downloads.sub.tentative.html.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=PgTvqcLAS2qlSUf7y2BNYw-0&resultStatus=testfailed%2Cbusted%2Cexception&revision=79046ff8143bf9dcd8cedff9ab6357a0611cc43c

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302469484&repo=autoland&lineNumber=4962

Backout: https://hg.mozilla.org/integration/autoland/rev/fe40ddb45918d471f6358c904e30d9667ce06975

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:sstreich, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb9de49e4f7d
Pass the TriggeringSandboxFlags to nsILoadinfo r=ckerschb,smaug,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/441ff87bf868
Block downloads in sandboxed iframes r=ckerschb,smaug

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/eb9de49e4f7d
https://hg.mozilla.org/mozilla-central/rev/441ff87bf868

Comment 19

[Sebastian Streich [:sstreich]]

Clearing ni as this now finally landed 😅

Comment 20

[j.j.]

Note:
sandbox=allow-downloads and others are not yet documented in the compat table
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe#Browser_compatibility

Comment 21

[Tom S]

https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/81$revision/1643705
https://github.com/mdn/browser-compat-data/pull/6668