Skip to content
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

adjustments to HostEnsureCanCompileStrings discard value #144

Closed
mikesamuel opened this issue Apr 3, 2019 · 2 comments
Closed

adjustments to HostEnsureCanCompileStrings discard value #144

mikesamuel opened this issue Apr 3, 2019 · 2 comments
Labels

Comments

@mikesamuel
Copy link
Collaborator

https://github.com/WICG/trusted-types/blob/1f0e41ff742de43e70261814ac2b2d314b630e1e/spec/index.bs#L1430-L1446

This uses Get Trusted Type compliant string which might apply the default policy.
This is the right thing to do so that applications can provide minimal workarounds for legacy code patterns like

const globalObject = new Function('return this;')();

which appears frequently in JS library code.


Uses in the JS spec are written like

1. Perform ? HostEnsureCanCompileStrings(evalRealm, evalRealm).

The return a value is never used: it either throws or returns normally.


This causes two problems:

  1. Any adjustments made by the policy are lost, so a policy like
    TrustedTypes.createPolicy('default', {
      createScript(x) { return rewriteJavascript(x); }
    });
    the value that will reach CreateDynamicFunction will be x, not rewriteJavascript(x).
  2. Any non-trusted types value will be stringified twice which will cause unnecessary
    visible side-effects for values like
    let stringifyCallCount = 0;
    f = new Function({
      toString() {
        return stringifyCallCount++ ? 'evil()' : 'good()'
      });
    
    which might cause the policy to see 'good()' but CreateDynamicFunction to receive 'bad()'.
@mikesamuel
Copy link
Collaborator Author

AFAICT, there's no fix that satisfies the const globalObject = new Function('return this;')(); use case and which does not require changing the nature of the JS / browser interactions.

@mikewest Do you have a sense of how browser implementors might react to replacing language like

  1. Perform HostEnsureCanCompileStrings(calleeRealm, callerRealm, goal, sourceText).

with

  1. Set sourceText to HostBeforeCompile(calleeRealm, callerRealm, goal, sourceText).

(tc39/ecma262#1498 defines goal and sourceText.)

@koto koto added the spec label May 4, 2019
mikesamuel added a commit to mikesamuel/trusted-types that referenced this issue May 9, 2019
This reworks how `new Function(source)` and `eval(source)` are
checked against a CSP and Trusted Types policy.

It trusts TrustedScript when the relevant CSP policies and TrustedType
configurations agree on whether to enforce and the TrustedType
configuration places limits on policy creation.

It also changes the previous language to use *calleeRealm* instead of
*callerRealm* for consistency with other sinks.

> ```js
>   let f = new self.top.Function(source);
> ```
> In this case, the *callerRealm*'s Window is `self` and the
> *calleeRealm*'s Window is `self.top`.
> The Trusted Types portion of this algorithm uses *calleeRealm*
> for consistency with other sinks.
> ```js
>   // Assigning a string to another Realm's DOM sink uses that
>   // Realm's default policy.
>   self.top.body.innerHTML = 'Hello, World!';
>   // Using another Realm's builtin Function constructor should
>   // analogously use that
>   // Realm's default policy.
>   new self.top.Function('alert(1)')()
> ```

It also makes recent versions of `bikeshed` run without warnings.

Fixes w3c#143
Issue w3c#144
mikesamuel added a commit to mikesamuel/trusted-types that referenced this issue Jul 1, 2019
This reworks how `new Function(source)` and `eval(source)` are
checked against a CSP and Trusted Types policy.

It trusts TrustedScript when the relevant CSP policies and TrustedType
configurations agree on whether to enforce and the TrustedType
configuration places limits on policy creation.

It also changes the previous language to use *calleeRealm* instead of
*callerRealm* for consistency with other sinks.

> ```js
>   let f = new self.top.Function(source);
> ```
> In this case, the *callerRealm*'s Window is `self` and the
> *calleeRealm*'s Window is `self.top`.
> The Trusted Types portion of this algorithm uses *calleeRealm*
> for consistency with other sinks.
> ```js
>   // Assigning a string to another Realm's DOM sink uses that
>   // Realm's default policy.
>   self.top.body.innerHTML = 'Hello, World!';
>   // Using another Realm's builtin Function constructor should
>   // analogously use that
>   // Realm's default policy.
>   new self.top.Function('alert(1)')()
> ```

It also makes recent versions of `bikeshed` run without warnings.

Fixes w3c#143
Issue w3c#144
@koto
Copy link
Member

koto commented Aug 11, 2019

Should this be tracked under #207 or the tc39 repos, or do we need this bug separately?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants