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

"Trusted types" and "Literals in script" #30

Closed
craigfrancis opened this issue Nov 17, 2017 · 7 comments
Closed

"Trusted types" and "Literals in script" #30

craigfrancis opened this issue Nov 17, 2017 · 7 comments

Comments

@craigfrancis
Copy link

In response to @mikewest
https://mobile.twitter.com/mikewest/status/931439227715321857

And how this "Trusted types" proposal might work with the "literals in script" proposal - which may be related to Issue 4...


If the JS engine could keep track of the variables that were created from String Literals (or String Constants), this could be very useful.

I rarely use HTML strings (e.g. for templating), but when I do, I'd only use a String Literal; any user supplied/tainted values will be applied to the DOM later, via "safe" methods like el.textContent or el.setAttribute(), while keeping in mind that certain attributes (e.g. <a href="xxx">) aren't exactly safe.

So I'd like there to a way to instruct the browser to only accept String Literals via unsafe methods like el.innerHTML... as in, anything tainted would be rejected.

This is kind of related to Issue 11, which suggests overriding these methods.

I think the extra level of needing to create a TrustedHTML object is far to messy/complex for most developers - where even I'm probably not going to update all of the JS on my websites to use these objects, or use new methods like safelySetInnerHTML().

Being completely selfish, if el.innerHTML = “literal”; was allowed, and anything else was blocked, I probably wouldn't need to change anything (other than instructing the browser to do this extra level of checking, and re-running the tests to make sure I hadn't missed something).


On a slightly related note, I'd be happy to use the CSP header to require-trusted-types, as this does relate to the Security Policy for the pages Content :-) ... it's similar to how you can opt-in to XSS Protection (issue 1).

I'm just wondering if a similar approach to "use strict"; could be used, only on the basis that a website will typically pull in several JS files, and it will be easier to apply this logic when each script is updated (i.e. piecemeal migrations).


For example, I think this should work (well, ignoring pointless use of JS):

<div id="members" data-name="Mike"></div>

<script>

    // This would typically be in an external/async script.

    ;(function(document, window, undefined) {

        'use strict';
        'use trusted-types';

        if (!document.addEventListener) {
            return;
        }

        function init() {

            var members_ref = document.getElementById('members'),
                members_html = '<p>Hi <strong></strong></p>';

            if (members_ref) {
                members_ref.innerHTML = members_html;
                members_ref.getElementsByTagName('strong')[0].textContent = members_ref.getAttribute('data-name');
                // Lets assume I've checked these are all defined.
            }

        }

        if (document.readyState !== 'loading') {
            window.setTimeout(init); // Handle asynchronously
        } else {
            document.addEventListener('DOMContentLoaded', init);
        }

    })(document, window);

</script>

But if I changed members_html to '<p>Hi <strong>' + tainted_name + '</strong></p>', then the browser would reject it for members_ref.innerHTML

@koto
Copy link
Member

koto commented Dec 1, 2017

Hey,

Thanks for the feedback. We are trying to make Trusted Types as easy as possible to adopt, if the security guarantees can be met - this is the case when assigning literals to innerHTML. That said, it's all blocked on https://github.com/mikewest/tc39-proposal-literals getting introduced. If the browser can't know from the JS engine what is a literal, that obviously we can't simply allow strings as trusted values for assigmnent.

For now it seems like the approach skews towards using the template literal tag functions (instead of being based on raw strings), so unfortunately the code would need updating to something like:
node.innerHMTL = TrustedHTML.fromTemplateLiteral(assertLiteral``xyz``).

I really would like node.innerHTML = 'literal' or 'node.innerHTML = literal' being possible for TrustedTypes, we're looking if that's possible, as that's a huge adoption boon.

As for using TT per-script, this is also an interesting approach, somewhat related to #31 script-specific unsafe creation blocking. There's one problem with this in that it's insecure by default, but maybe we can have both? use trusted-types would enforce the types for a given script, and CSP require-trusted-types would enforce it for all scripts in a document. This way of triggering the types has the problem of being fairly invasive to JavaScript itself (think how much time was spent in specifying and implementing strict mode), so it might be a really tough sell to have it.

@craigfrancis
Copy link
Author

Thanks @koto, and yes, it would be blocked on the proposal made by Mike West... it was his proposal that led me to this project, and the hope that the two could work together (tbh, I doubt I'd be re-writing my JavaScript to use things like TrustedHTML.fromTemplateLiteral).

And I'd agree that a per-script opt-in is not safe by default. So having something in the CSP to enforce it would hopefully solve that issue - this would mean websites could work though their existing scripts to introduce Trusted Types individually, when they believe they have them all they could use CSP-Report-Only (to see if they get any reports), then move it into a site-wide requirement (either via the pages CSP, or via Mikes suggestion of eventually having a site/origin-wide CSP).

@koto
Copy link
Member

koto commented Dec 1, 2017

Yes, I see the point of this, I extracted it to #33.

@koto
Copy link
Member

koto commented Jun 4, 2018

Shelving this for now, this didn't get enough traction yet in tc39. I'll reopen when the time is right for this :)

@koto koto closed this as completed Jun 4, 2018
@craigfrancis
Copy link
Author

As the tc39 literal thing might not happen (variables being tracked to see if they're only made up of literals), could the browser detect the obvious, no variable in use, and white-list it?

Admittedly this would be hard to polyfill.

The reason I ask is because most of my work to adopt Trusted Types has been converting:

link.setAttribute('href', '#');
img.setAttribute('src', '/a/img/loading.gif');

To using:

link.setAttribute('href', trusted_types.createURL('#'));
img.setAttribute('src', trusted_types.createURL('/a/img/loading.gif'));

Then adding this to the start of my scripts:

var trusted_types = {
    'createURL': function (s) {
        if (s.substring(0, 7) == '/a/img/') {
          return s;
        }
        return '#';
      }
  };

if (window.TrustedTypes) {
  trusted_types = TrustedTypes.createPolicy('example', trusted_types);
}

@koto
Copy link
Member

koto commented May 13, 2019

The browser alone cannot detect this (whether a function parameter is a literal or a variable), this would have to be a JS language interpreter feature - and we're working on it in #96. In the meantime, only workarounds are possible:

One of them is to rely on linter checks to assure that a given policy createXYZ functions are only called with literals.

Another is to rely on typing annotations in TypeScript. https://github.com/koto/web/blob/trusted-types-angular/src/trusted-types/const.ts#L11 demonstrates a type that represents a literal string. With that you can write functions that can only be called with literal strings (or else the code does not compile). Demonstration of this approach is at https://youtu.be/il30oOJ2bWk?t=363.

In JavaScript you can make assertions similar to that: For example, write a tag function

function createURLFromLiteral(strings, ...args) {
  if (args.length || strings.length > 1) {
    throw 'Interpolation!'
  }
  return aPolicy.createURL(strings[0]);
}

and then replace all sink = 'literal'; with sink = createURLFromLiteral`literal`; (you can make automatic refactoring for this).

All of those workarounds are bypassable until the language can let us distinguish a literal from a variable string, but if you don't assume your developers are maliciously trying to add XSS to your applications, this should be fine.

@craigfrancis
Copy link
Author

Thanks @koto, I keep forgetting the JS interpreter is very much its own thing.

For now I'm happy to keep using my simple ES5 compliant approach, as it's only a few extra lines of JS at the start, and then updating/checking the lines that set a static href/src (the bit I was hoping to skip).

I'm just being a grumpy git in not switching over to TypeScript, or using an ES6 transpiler - where I didn't know you could make tagged template strings, I assumed it was just for creating a string, which I thought was horrible from a security point of view:

function my_template(strings, ...values) {
  console.log(strings);
  console.log(values);
}
url = 'javascript:evil()';
console.log(`<a href="${url}">link</a>`); // Nope, bad, do not use with innerHTML.
my_template`<a href="${url}">link</a>`; // Interesting.

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

No branches or pull requests

2 participants