Skip to content

Have Trusted Types API built directly into the jQuery Core Files #4409

Closed

Description

The latest version of jQuery V3.4.1 has 5 innerHTML() which are not sanitized by default. Does the jQuery team plan on addressing these DOM XSS things in the near future with regards to the new Trusted Types API see here: https://github.com/WICG/trusted-types

Activity

  1. mgol commented on May 27, 2019

    @mgol
    Member

    Thanks for the report. What would you expect jQuery to do here?

  2. mgol commented on May 27, 2019

    @mgol
    Member

    Note, though, that jQuery doesn't support experimental APIs as they may change and then we're left with obsolete code. It's fine to experiment & think of the approaches but we'd wait with landing any actual code in the library for the spec to stabilize & to be implemented in at least 2 major browser engines.

  3. dmethvin commented on May 27, 2019

    @dmethvin
    Member

    Yeah, this is a tough one. The team has discussed it for years and had issues determining how to make things safer without seriously redesigning the API. Most of our HTML goes through jQuery.parseHTML() and jQuery.htmlPrefilter() so it is currently possible to try and sanitize through that. There's the one fast path for .html(string) that assigns directly to .innerHTML but that already goes through the prefilter and could just be taken out if needed. The others are Sizzle feature checks for browsers we won't support in 4.0 anyway.

    Perhaps we could recognize a Trusted Type and pass it through as if were HTML, but not actually do any manipulation with it, e.g. $("div").append(someTrustedType). (As an aside, I would hope that a Trusted Type doesn't return an HTML representation when you call .toString() on it.)

    Seems like they've taken old browsers into account with a simple polyfill that just returns strings if the browser doesn't support Trusted Types.

    But given where the implementations are at, it is probably too early to add any code.

  4. added this to the 4.0.0 milestone on Jun 3, 2019
  5. mgol commented on Jun 3, 2019

    @mgol
    Member

    I added it to the 4.0.0 milestone so that we don’t forget about it when we get closer to the release.

  6. timmywil commented on Jun 10, 2019

    @timmywil
    Member

    I'm in favor of this. We'll keep an eye on the spec status and revisit later.

  7. mgol commented on May 26, 2020

    @mgol
    Member

    angular/angular.js#17028 has some code that I'd like to backport to jQuery, at least to the master branch. It might be hard to do on the 3.x line as IE 9 needs the existing approach so we'd have to fork logic, increasing the size significantly which we usually try to avoid.

  8. self-assigned this
    on May 26, 2020
  9. koto commented on May 27, 2020

    @koto

    Bringing in the comment from angular/angular.js#17028

    I've been thinking about including similar improvements [Trusted Types support] to jQuery. AngularJS has included code specific to APIs not available cross-browser like Chrome apps and now possibly trusted types but jQuery has historically avoided such things. Avoiding innerHTML for wrapping like done in this PR would work but we wouldn't want to mention the TrustedHTML constructor explicitly as long as it's a Blink-only feature.

    To clarify, TT are intended to be rolled out as a regular web API, but you're correct - for now this is only implemented in Blink (and polyfilled in other browsers).

    Do you think there's any other way to support this use case without mentioning any trusted types API explicitly? I'm asking here as if it's possible maybe the same technique could be applied here. If you prefer to continue this part of the discussion on the jQuery side, there's an open issue about trusted types support: #4409.

    In general most of jQuery APIs seem to fit well into what we call "tt-agnostic" bucket. In other words, they should be able to accept TT instances (in place of strings) and pass them without modification to DOM.

    Some of jQuery behavior violates that. For example, some of the tag wrapping code + self-closing tag expansion that was just changed due to XSS issues. Large parts of these patterns could be gated on browser checks, especially if they workaround some legacy browser behavior.

    Some things still remain though, e.g. jQuery IIRC still expands <script> nodes and tries to execute them separately -- and in general jQuery APIs are quite polymorphic, trying to detect the parameter type and do the appropriate thing (jQuery()function for example is very much like that). This paradigm may make it difficult to untangle and simply add support for a new value type, as you might often collide with types already passed to your functions in the wild.

    Usually there is a way to become tt-agnostic with no backward compatibility issues without referencing TrustedTypes at all, as DOM APIs are very rarely called with Objects, and Trusted Type instances are Objects. Unfortunately, jQuery offers a very limited API for jQuery(plainObject) case. The only way I see is to not recognise TrustedTypes as isPlainObjects and introduce a branch for "immutable HTML objects" in jQuery(), opting them out of modifications. But this requires referencing TT.

    Adding such support should not cause b/w compatibility issues, as it's simply a new way of using jQuery APIs (one that happens not to violate TT restrictions that might be present in your CSP), that some applications may use. I'm confident that with relatively small changes we might be able to support most use cases.

    That said, I'm a bit skeptical all applications using jQuery could migrate to "TT mode" by a single configuration switch. Quite likely some jQuery functions would have to change the behavior, error out, or produce types internally when called with Trusted Types (script node reparsing comes to mind). It's a judgement call on how to treat each of the cases like this. For example, perhaps jQuery customer could configure it with a policy to call when it needs to change HTML - giving users a hook to sanitize the resulting HTML. You could call it a htmlPostFilter function and not reference TT there at all. But some things (e.g. cross-frame issues) will likely remain incompatible with TT, and would need a change in the application.

    One important thing to note is that TT should not be treated as "better strings for HTML", and you should never return a Trusted Type when the caller expects a string - we got bit by this when making DOMPurify.sanitizereturn a TrustedHTML, that broke projects that called DOMPurify.sanitize(foo).replace - a method that does not exist. But jQuery, to my understanding should not return a Trusted Type, but rather accept it (or, perhaps produce it and use internally).

    I'm happy to help with fleshing out the integration, or even brainstorming about issues with one. While I don't have tons experience with recent jQuery, we have done our share of TT integrations already and know some best practices and antipatterns. What would be the next best steps?

  10. mgol commented on May 29, 2020

    @mgol
    Member

    @koto Thanks for a detailed comment!

    In general most of jQuery APIs seem to fit well into what we call "tt-agnostic" bucket. In other words, they should be able to accept TT instances (in place of strings) and pass them without modification to DOM.

    Some of jQuery behavior violates that. For example, some of the tag wrapping code + self-closing tag expansion that was just changed due to XSS issues.

    Fortunately, that function has been changed to an identity function in jQuery 3.5.0 so it'd no longer break a trusted type wrapped HTML.

    Large parts of these patterns could be gated on browser checks, especially if they workaround some legacy browser behavior.

    There's an important aspect here. jQuery generally avoids browser detection, instead opting for support tests. The master branch doesn't contain a lot of them right now as most were meant for IE which we now just detect via document.documentMode but that's the only browser sniffing we accept (as IE is feature-dead so the risk is low). Those feature tests sometimes need to use innerHTML-based assignments, see e.g. this test for a Safari 8 issue:
    https://github.com/jquery/jquery/blob/3.5.1/src/core/support.js#L13-L17
    While we don't have many tests like that - and none on master at the moment, we can't be sure there won't be a need for a new one in the future.

    We already had to be careful here due to CSP issues which we test for:
    https://github.com/jquery/jquery/blob/3.5.1/test/unit/support.js#L40-L55
    but it doesn't block innerHTML as a whole which Trusted Types enforcement would do.

    I feel this is a more generic issue with CSP, though: what we need is a way to run support tests in a secure way that's not blocked by CSP. The environment where we run them could be isolated from the real webpage in which case there would be no security risk even if we used inline code, raw HTML strings, etc. Is it likely we'll get something like that?

    Some things still remain though, e.g. jQuery IIRC still expands <script> nodes and tries to execute them separately

    That's a good argument for us to stop doing that by default in jQuery 4. This would be a significant breaking change, though, so we'd still need to add a non-default way to keep the old behavior.

    Usually there is a way to become tt-agnostic with no backward compatibility issues without referencing TrustedTypes at all, as DOM APIs are very rarely called with Objects, and Trusted Type instances are Objects. Unfortunately, jQuery offers a very limited API for jQuery(plainObject) case. The only way I see is to not recognise TrustedTypes as isPlainObjects and introduce a branch for "immutable HTML objects" in jQuery(), opting them out of modifications. But this requires referencing TT.

    This API only accepts plain objects, though. We could perhaps make that explicit & treat non-plain objects in a different manner. The only issue is that the jQuery() function mostly serves two purposes: as a shortcut of jQuery(cssSelector) to $(document).find(cssSelector) and as a mechanism to create new elements representing a provided HTML string which more or less depends on whether the string starts with <, possibly preceded by whitespace:
    https://github.com/jquery/jquery/blob/3.5.1/src/core/init.js#L36-L41
    or:
    https://github.com/jquery/jquery/blob/3.5.1/src/core/init.js#L50-L51
    This is such a fundamental property of the library that it's impossible to give up on this polymorphism. Maybe it's not an issue since trusted HTML stringifies to the underlying HTML, though, we'd just need to cast it to string before running the checks?

    I feel a bit uneasy to explicitly mention the API in jQuery source; we've seen it a few times that APIs only supported by one browser may disappear or change significantly before being accepted accross all the major engines and then we'd often have to support both the old & new APIs. I'd like to first research how far we can go without relying on the API explicitly.

    I'm happy to help with fleshing out the integration, or even brainstorming about issues with one. While I don't have tons experience with recent jQuery, we have done our share of TT integrations already and know some best practices and antipatterns. What would be the next best steps?

    I think I'll first backport the AngularJS changes to jQuery master & then the next step would be to try to fix the jQuery() method to not break on trusted types via means I described above. For now I'd focus on the master branch; if we make it there, we could consider whether that's possible to backport to the 3.x line, although I'm a bit skeptical here.

    It'd also help if we had an answer for how to handle support tests in a secure, CSP-compliant way; in particular, if we can get any browser APIs that would allow them to run in a sandboxed environment even with CSP enabled.

  11. 24 remaining items

  12. zwjohn commented on Sep 15, 2021

    @zwjohn

    @koto thanks for sharing your solution, I will take a look. I could use DOMPurify library to purify the strings, however, I think Jquery should handle this in the library or replace usage of innerHTML to avoid this error, otherwise, every developer using Jquery will run into this issue sooner or later.

  13. added 4 commits that reference this issue on Sep 17, 2021
    c43d1ee
    9af57b1
    566171b
    e3165e6
  14. added a commit that references this issue on Sep 30, 2021
    de5398a
  15. mgol commented on Sep 30, 2021

    @mgol
    Member

    This has landed in #4927 and will be included in jQuery 4.0.0. Thanks for all the discussions!

  16. koto commented on Sep 30, 2021

    @koto

    Thank you Michał, that's great news!

  17. koto commented on Oct 7, 2021

    @koto

    I noticed one violation still in jQuery.attr, where the value of the attribute is stringified before calling setAttribute:

    const policy = trustedTypes.createPolicy('a', {createScriptURL: s=>s})
    jQuery(aScript, policy.createScriptURL('foo'));
    
    Uncaught TypeError: Failed to execute 'setAttribute' on 'Element': This document requires 'TrustedScriptURL' assignment.
        at attr (jquery.js:6997)
        at access (jquery.js:3467)
        at jQuery.fn.init.attr (jquery.js:6955)
        at <anonymous>:1:6
    

    There is a workaround via .attr hooks, but it might be worth addressing nonetheless.

    It seems like this was introduced in ff75767, I suspect to workaround an IE <= 9 bug, which incorrectly stringified objects passed to setAttribute.

    I'm not sure what the most elegant solution would be here, I guess it depends on whether jQuery 4 aims to support IE9. If not, it's safe not to stringify values (browser API would). If yes, then there's only a less-than-ideal option of testing for the bug? IIRC this would be a good test:

    with (document.createElement('div')) {
      setAttribute('title', {toString:()=>''}); 
      getAttribute('title') === ''
    }
    
  18. mgol commented on Oct 7, 2021

    @mgol
    Member

    @koto Thanks for the info. jQuery 4.x will only support IE 11 so if this was just to please IE <=9 we should be good to remove the manual stringification.

  19. mgol commented on Oct 7, 2021

    @mgol
    Member

    @koto I created #4948 out of your last comment.

  20. locked as resolved and limited conversation to collaborators on Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions