Skip to content

Include the string to be compiled in the call to HostEnsureCanCompileStrings #938

Open

Description

@mikewest

To improve the quality of CSP reports, it would be helpful for HostEnsureCanCompileStrings() to include the string to be compiled as an argument. HostEnsureCanCompileStrings(callerRealm, calleeRealm, source) seems ideal. :)

The goal is to ensure that we can include a sample of the script which violates the policy when generating a CSP violation report. We're doing this for inline <script>...</script> blocks today, and layering eval() and the like on as well would be helpful.

Activity

  1. domenic commented on Jun 21, 2017

    @domenic
    Member

    Sounds good in theory. The only reason we didn't do this at the time was because it wasn't needed, IIRC.

    There are a couple of tricky things:

    • How do you want to handle non-strings? Currently in a CSP-restricted environment, eval(nonString) will throw. (I wonder if we have tests for that?) One refactoring would be to ensure things are a string before passing them to HostEnsureCanCompileStrings. If we do that, then behavior will change, and we'll bail out before ever hitting HostEnsureCanCompileStrings.
    • Similarly, for Function() and its various friends (GeneratorFunction(), AsyncFunction()), do we handle arg coercion before or after HostEnsureCanCompileStrings? This would change which error is thrown when doing e.g. Function({ toString() { throw 5; }).
    • For Function() and friends, what text should be passed? Should it be the body text of the function, which is directly passed as an argument? Or perhaps we should re-serialize the entire function, after we've assembled the various arguments to Function() into an actual function? The former would probably work for your purposes, although it's a bit weird to be doing checks on something that isn't standalone evaluatable source code. Maybe it would help if we named the new parameter something like sourceTextHint or proximateSourceText instead of just sourceText, with a reasonable explanation.
  2. koto commented on Jan 18, 2019

    @koto

    This issue also surfaced when creating the trusted types spec draft. In short, we're trying to figure out if eval could accept stringifiable objects, and the decision in HostEnsureCanCompileStrings would be based on their value (essentially, we would optionally reject all values that are not of a given type).

  3. koto commented on Jan 24, 2019

    @koto

    Actually, having looked at it, for Trusted Types in specific, we might need to be able to validate and coerce a given 'eval' argument to a string before e.g. PerformEval is called. The issue is that eval returns its input value if it's not a string:

    eval(new String('foo'))
    String {'foo'}
    

    The use case we have in Trusted Types is to be able to, in the host environment, allow

    eval(ASpecificObjectWrappingAString("code-to-compile"))
    

    but optionally disallow:

    eval("code-to-compile")
    

    or even transform it to :

    eval("modified-code-to-compile")
    

    Theoretically, this can still be done in HostEnsureCanCompileString if it accepted an object passed by the caller as-is and returned a tuple of boolean value and the (potentially coerced to string) code to compile. Spec-wise this would require small changes to a few HostEnsureCanCompileString callsites, namely:

    I'm not sure how involved the change is, both in spec and the implementations.

  4. mikesamuel commented on Jan 24, 2019

    @mikesamuel
    Member

    Similarly, for Function() and its various friends (GeneratorFunction(), AsyncFunction()), do we handle arg coercion before or after HostEnsureCanCompileStrings? This would change which error is thrown when doing e.g. Function({ toString() { throw 5; }).

    Excellent question.

    1. If coercion happens too early, then HostEnsureCanCompileStrings lacks context to distinguish between a trusted script and a raw string.
    2. If coercion happens after CanCompileStrings, then a HostEnsureCanCompileStrings implementation can't guarantee that the stringified form observes is the same as the stringified form that the parser sees.

    It seems that for mkwst's diagnostic purposes, (2) is not a breaker, but for TT, (1) is a breaker.

    Per the questions about functions, those seem like non-issues since, as koto points out, HostEnsureCanCompileStrings is never called.

    $ npx node@10 --disallow_code_generation_from_strings -e 'console.log(eval(() => {}))'
    [Function]
    
    $ npx node@10 --disallow_code_generation_from_strings -e 'console.log(eval("() => {}"))'
    [eval]:1
    console.log(eval("() => {}"))
            ^
    
    EvalError: Code generation from strings disallowed for this context

    Perhaps we could tweak the eval algo so that if the input is a string or has a particular symbol property then it is stringified to avoid breaking code that depends on the current behavior where eval is identity except for strings.

  5. koto commented on Aug 11, 2019

    @koto
  6. ljharb commented on Aug 11, 2019

    @ljharb
  7. mikesamuel commented on Aug 11, 2019

    @mikesamuel
  8. added
    proposalThis is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.
    on Jan 3, 2020
  9. Jack-Works commented on Aug 19, 2022

    @Jack-Works
    Member

    Without the source, the host cannot implement the following CSP:

    Content-Security-Policy: script-src 'sha256-0X/mXwSMKUkzUfv+VIF49SFUDL0crPAnC532AAN4J74=' 'sha256-85Tr60VsxQkO/J0dtiQqqhbd1LXNQ/Mwu1wDTDY9jVo=' 'sha256-dRXLIytYygaF4gDr8/bnkhxCVOOZIXq6TsUYkPu2COM=' 'sha256-FtWtxCFT2E9x4YfsMlTkUyFn0iWdCkat4/Ug6ESriuo=';
    
    eval(`  // External script, using by test 12, 8, 26, 135, 158, 160/sha256
    document.getElementById("test_external_script").innerHTML = "Hi, I am script from external test.js";`)
    

    And it cannot implement TrustedTypes either. The host needs to get the compiling source to determine if they should reject or allow this compilation.

  10. caridy commented on Aug 19, 2022

    @caridy
    Contributor

    @Jack-Works I agreed with you, in fact I was very surprised yesterday when looking at this. Maybe an implementer can provide more details about how this works today with trusted types? cc @mikesamuel

  11. koto commented on Aug 19, 2022

    @koto
  12. nicolo-ribaudo commented on Feb 22, 2024

    @nicolo-ribaudo
    Member

    The hook now receives the source: #3222

  13. lukewarlow commented on Mar 14, 2024

    @lukewarlow

    @nicolo-ribaudo I'm not sure this is actually addressed by that change. That change provides the strings separately but not the compiled version which is needed for CSP reports. This will be addressed by #3294 however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    proposalThis is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions