Consider allowing string values in CSSResults?

[43081j]

Currently css only accepts CSSResult values (other css calls) which means we can no longer do something like this:

static get styles() {
  return [css`
    :host {
      background-image: url('${rootPath}images/foo.png');
    }
  `];
}

This is a common pattern I have throughout my project to handle usages of import.meta so assets can have absolute paths.

If you can suggest a better way to handle passing in a path, fair enough, but otherwise it would be handy to be able to interpolate string values here.

[justinfagnani]

This would be a potential security vulnerability.

[43081j]

What do you suggest for this use case? It seems like it should be a common one.. wanting to use the meta url in paths.

[justinfagnani]

cc @rictic @mikesamuel

[sorvell]

This should work as long as you use css to define rootPath. This ensures it's a literal from script and addresses the security concern.

const rootPath = css`...`;

[LarsDenBakker]

Since rootPath will likely be generated from a variable, its not possible to create it through a css tag. css${import.meta.url} throws an error.

[43081j]

^ exactly this @sorvell

its coming from import.meta, we don't define it. as people move more towards using import.meta i imagine this'll become a more common use case too.

FYI you can work around it with new CSSResult(import.meta.url)

[web-padawan]

Note: here is how I'm using the CSSResult at the moment to get data from <dom-module>:

import { DomModule } from '@polymer/polymer/lib/elements/dom-module.js';
import { stylesFromTemplate } from '@polymer/polymer/lib/utils/style-gather.js';
import { CSSResult } from 'lit-element';

export const getStyleModule = (id, cb) => {
  const template = DomModule.import(id, 'template');
  let cssText =
    template &&
    stylesFromTemplate(template)
      .map(style => style.textContent)
      .join(' ');
  // callback to process cssText
  // use case: rewrite tag name for slotted children:
  // example: vaadin-item -> lit-item
  if (cb) {
    cssText = cb(cssText);
  }
  return new CSSResult(cssText);
};

The result is added to the styles like this:

static get styles() {
  return [...super.styles, getStyleModule('lumo-button')];
}

See https://github.com/web-padawan/lit-components/tree/master/packages/polymer-style-module

I really hope the export for CSSResult won't be removed as it will break my code 😃

[rictic]

Hm. When running in secure mode, we could add a check that forbade strings but allowed TrustedResourceUrls. I'm not sure how you'd build a TrustedResourceUrl out of import.meta.url though. You might need a new path with compiler checks to validate it.

[jsilvermist]

Why is this an issue with css but we can interpolate strings in html?

[LarsDenBakker]

Interpolated strings in html are set as textContent, not parsed as HTML. So there is no security risk there. With css there is, but it is unfortunately a big downside for developer experience :(

[justinfagnani]

This could simply be a case where you need to use styles inline with the template for now until we figure out how to feed import.meta.url safely into CSS without opening a hole for arbitrary strings.

I hear there's work on trusted types in Chrome, so there may be a longer-term path forward there.

[bashmish]

In LitElement you define your styles via a static getter. This does not allow you to use element's instance properties which contain dynamic data which often comes from the untrusted user input. You compose you final styles once on a class level. I don't see where you will get untrusted user input there, you will likely always compose your static styles from other ES modules and just static values originating from your source code.

So I think the problem is exaggerated a lot. It certainly exists theoretically, but how often? I think the solution is unbalanced: it fixes the theoretical issue while breaking a very frequent use case.

[tlouisse]

I also would like to use css variables that are reusable in many contexts, not being coupled to CSSResult per se.

For instance: I want to be able to define my (grid) breakpoints as a single source of truth throughout my app, and reuse them in both css and js:
https://stackblitz.com/edit/lit-element-css-usecase?file=css-test.js
(using these variables: https://stackblitz.com/edit/lit-element-css-usecase?file=variables.js)

Or, more generically speaking: I might want to be able to combine my globally defined variables in other contexts (for instance combine it with other technology stacks) that do not rely on CSSResult. It would be nice if I could do that without wrapping all my vars in css``.

Also, in some situations with a lot of small nested css strings (for instance when creating functions comparable to Sass Mixins) it feels a bit cumbersome to wrap every small string in css. It would also be a bit less performant.

Alternative to CSSResult
Instead of using CSSResult, one would also be able to use css as a function and pass in a string array as its first parameter:

const myBreakpoint = 400;
const myStyles = css`
  @media (max-width: ${css([myBreakpoint])}px) {
     ...
  }
`

This would also mean that usage of css by itself doesn't solve a security issue? Since you can do:

const myBreakpoint = somethingPotentiallyMaliciousProvidedByUserInput;

Taking this into account, it would be the responsibility of the developer to make sure there would be no security issues, not the css function.

Sanitization?
The security argument would mainly make sense to me if the css function would do sanitization.
I don't know much about adoptingStylesheets, but would it happen here?

     if (supportsAdoptingStyleSheets) {
        this._styleSheet = new CSSStyleSheet();
        this._styleSheet.replaceSync(this.cssText);
      }

https://github.com/Polymer/lit-element/blob/master/src/lib/css-tag.ts#L30

[mikesamuel]

Why is this an issue with css but we can interpolate strings in html?

In CSS, the risk is mostly privacy violations.

There's always injecting a background URL that's a transparent image that phones home to a server controlled by the attacker.
Depending on Referrer-Policy this can leak quite a lot.

The other risk is stealing data from the page including from form inputs.
https://www.mike-gualtieri.com/posts/stealing-data-with-css-attack-and-defense is a pretty good writeup on that.

Ideally, URLs in CSS would all be specified as SafeURLs and coerced to a known format like url("...").

---

This is a common pattern I have throughout my project to handle usages of import.meta so assets can have absolute paths.

Ok, so you already trust the origin import.meta to load script from, so why not trust it for images and fonts too?

Do JS bundlers rewrite import.meta.url to a string literal? For example if I do npm install https://github.com/...?
If so, clients of a library that does this might think they're loading assets from a CDN they control, but images and fonts are coming from the package repository.

[43081j]

Ok, so you already trust the origin import.meta to load script from, so why not trust it for images and fonts too?

I do. I don't remember saying I don't, so not sure I quite follow you here...

Do JS bundlers rewrite import.meta.url to a string literal? For example if I do npm install https://github.com/...?

It gets rewritten to a path relative to the output bundle afaik, with static assets output alongside it (as well as other chunks).

[mikesamuel]

@43081j

I do. I don't remember saying I don't, so not sure I quite follow you here...

Sorry for the confusion. I was asking to see if we're reasoning along the same lines.

It gets rewritten to a path relative to the output bundle afaik, with static assets output alongside it (as well as other chunks).

Thanks for explaining.

[justinfagnani]

@mikesamuel @rictic Would it be feasible to allow strings if we fed the bindings through the sanitizer with a fake<style> as the parent element?

[rictic]

Seems reasonable to me. I'm not sure what values are allowed as style text. Is there a TrustedStyleContent type?

[sorvell]

See #471 (comment).

[mikesamuel]

@justinfagnani Seems reasonable. Which sanitizer are we talking about though?