-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider dropping declarative scope for subresource loading. #580
Comments
I was thinking of a There's a suggestion in https://github.com/WICG/webpackage/blob/master/explainers/subresource-loading.md#link-based-api that a I would appreciate more opinions from security folks. :) |
The problem here is that this introduces another declarative way of altering the script loads like It's not very clear that, for example this code: const foo = document.createElement('link')
foo.rel = userparams.rel
foo.href = '//legitimate.example/' + userparams.bar;
document.head.appendChild(foo) is a gadget that used to be OK, but now can alter how the scripts are loaded on the page. This is surprising to code reviewers, introduces new attack surface that existing security tools don't cover -- and might lead to security bugs. It's not expected for On the other hand, import maps use a An even better solution would be to require Javascript execution to alter script loading via bundles - either partially, for example like outlined in https://github.com/WICG/webpackage/blob/master/explainers/subresource-loading.md#no-declarative-scope, or fully, by only allowing to fetch a webbundle from Javascript. That gives guarantee that a limited HTML injection (e.g. if the attacker can only insert certain tags, as others are filtered out) cannot lead to a security issue via webbundles.
That is one way of how the code that is executed in the document is altered by web bundles - this one without a need to access to any signing keys. Being able to disable selective script loads is problematic, as some scripts might perform additional security checks, workaround bugs, polyfill Web APIs etc. - and in general it gives the attacker another lever to trigger xs-leaks. |
Thinking about how bundles could be used, I think it's possible that they don't need to be loadable declaratively. In any case where you might want to reference a web bundle in the HTML which would contain a resource which you are authoritative for, you could instead serve a web bundle as the top level page. So perhaps we would not be losing any important functionality if we made these be only loadable through JavaScript? |
It looks like adding a However, I'm gradually getting more worried about sites that:
No matter whether the |
Don't these issue go away if there's no |
They would, but we'd wind up with a preloading interface that's very different from what HTML has used in the past. I'll ask the webperf mailing list for opinions. |
Bumping this up, did we achieve any consensus on this? @koto @jyasskin webbundle seems indeed very similar to But strictly from the security perspective, we've also had (still have actually) a similar mechanism I am more leaning towards @koto statement that this is just too risky, even with these strict conditions because it was proven in the past that these restrictions can often be fulfilled.
|
I was thinking more about the issue and I thought about a potential solution. What do you think? Are |
I'm a little worried about disabling header-based resource hints when it comes to bundle-based subresource loading. Wouldn't it be most useful if we had a serialization which worked for both HTML and HTTP headers, so that the fetch to the bundle could run ASAP? We could do this by using Link: headers but disabling the |
We haven't found consensus on this yet. I believe the path-based restrictions alluded to here and here convinced @koto that this wasn't a blocking issue, so the plan was to do an origin trial with The webperf list was friendly to using
|
I know sanitizers that allow |
We are switching to Although the subject of this issue is "Consider dropping declarative scope for subresource loading" and |
https://github.com/WICG/webpackage/blob/master/explainers/subresource-loading.md declares how, essentially, adding a link tag will change how, and if, some scripts on the page are being loaded.
Adding that primitive to the platform makes it harder to protect existing applications from XSS, as it bypasses existing methods that guard script loading (and likely server-side XSS mitigations like XSS filters are unaware of those new vectors as well).
For example, right now there's no requirement to have a CSP nonce in
<link rel="webbundle">
, and modifying the bundle scope in javascript does not require a Trusted Type, and yet adding that element may, IIUC, cause arbitrary scripts to get loaded, or prevent them from being loaded.New mechanisms that alter script loads should be programmatic, and not declarative.
/ cc @arturjanc @mikewest @lweichselbaum
The text was updated successfully, but these errors were encountered: