validation of per-element / per-attribute contents

[mozfreddyb]

@craigfrancis wrote in #18.

I'd prefer the per-element custom config, rather than separate lists for tags and attributes.

But could it go a little further, so the attributes can specify the allowed content?

Short example:

{
  'div': {
      'class': 'css-classes'
    },
  'a': {
      'href': ['url', 'mailto'] // Not inline javascript
    },
  '*': {
      'title': 'string'
    }
}

Why add this extra complexity?

1. Some attributes can take safe and unsafe values, e.g. the a[href] accepting "javascript:"

2. You could give hashes for acceptable inline JS/CSS - personally I wouldn't allow 'unsafe-inline' code, but I have to accept that many websites do.

3. If unsafe attributes get added to the spec in the future, it would be nice for the developer to have clearly stated if they intended it to be a custom attribute (not realising that data-* attributes exist), or if they intended to use the newly introduced attribute. For example, they might have <input ontap="myclick" /> for their JS to read and work with, and a theoretical future browser could see that as some JS to run.

This wouldn't be used by most systems, but for a theoretical complicated example:

{
  'a': {
      'href': [
          'url',
          'mailto',
          'sha256-SOMTvqVfOViiCfDUw29p76/OVddUs0V7HyE0bATK3K8=' // For: javascript:alert()
        ],
      'onclick': [
          'sha256-ZlNiuEKoYsR3vT5/phF5QQzmxOHlto0Qb7NgKx0WwV8=' // For: window.open(this.href); return false;
        ],
    },
  'input': {
      'ontap': 'string', // Arbitrary string, if this becomes an unsafe attribute, the value is dropped.
      'onfocus': 'unsafe-inline', // Arbitrary JS, danger, danger, avoid.
      'onclick': [
          'sha256-biFQTroSCI3Z5BmsMGyEE2jFZdwjjG1Oe7JLytgH6jM=', // For: this.select()
          'sha256-hphOYdb9WX9dW4pYcQdXa8E450mGtzl7k4kSIg1GOIo='  // For: this.value=''
        ],
      'style': 'unsafe-inline', // Might need some more thoughts on this one.
      'size': 'number'
  }
}

Where 'string' and 'unsafe-js' are effectively the same, but 'unsafe-js' will always be let though (the developer has explicitly stated they are happy with this risk, and it's easy for auditors to find); whereas 'string', will only be let though if the browser knows the attribute can accept this as a safe value (on the basis that the attribute may change in the future, or be added to the spec).

[mozfreddyb]

I could imagine an event-based validation approach similar to DOMPurify's event hooks, but I'd like to delay this discussion, until we're done with prototyping.

[otherdaniel]

I agree we'll need something better than allow/block, eventually. I'd prefer to leave this to a v2.

Declarative vs callback: IMHO, a declarative value spec - as in this proposal - has very nice security properties, but also tends to be a bottom-less pit in terms of complexity because there's always another new use-case that isn't quite covered by the existing facility. Callbacks are more flexible and more web-by, but IMHO more difficult to reason about. No idea what's best.

[koto]

A common usecase for URL validation in sanitizer is to either define a set of domains you allow the URL to point to, and to rewrite the URLs to proxy the request through a 1st party server for privacy concerns (e.g.. not disclosing the IP of the user).

…

On Mon, Jul 6, 2020 at 4:01 PM Daniel ***@***.***> wrote: I agree we'll need something better than allow/block, eventually. I'd prefer to leave this to a v2. Declarative vs callback: IMHO, a declarative value spec - as in this proposal - has very nice security properties, but also tends to be a bottom-less pit in terms of complexity because there's always another new use-case that isn't quite covered by the existing facility. Callbacks are more flexible and more web-by, but IMHO more difficult to reason about. No idea what's best. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA7JK7GHJ6Y3ZTESSPC5BLR2HKJ5ANCNFSM4ORRYZHA> .

-- koto@ / Krzysztof Kotowicz / Google

[cure53]

The above does not strike me as a task for a sanitizer.

[mozfreddyb]

Well, it is something folks are already doing with CSP, so I'd rather move this to "later".

[craigfrancis]

@cure53, I assume you're talking about URL validation?

If you're talking about the list of allowed tags, their attributes, and their values... what's considered safe/allowed varies; e.g. I cannot allow any inline styles (52pt red text would go against brand guidelines), but I might allow some known-safe class values (my light grey text being incorrectly used on a white background would cause accessibility issues), and in some locations I might allow <a target="_blank"> (keeping in mind the noopener issue).

[cure53]

@cure53, I assume you're talking about URL validation?

If you're talking about the list of allowed tags, their attributes, and their values... what's considered safe/allowed varies; e.g. I cannot allow any inline styles (52pt red text would go against brand guidelines), but I might allow some known-safe class values (my light grey text being incorrectly used on a white background would cause accessibility issues), and in some locations I might allow <a target="_blank"> (keeping in mind the noopener issue).

We were referring to @koto 's use case.

A common usecase for URL validation in sanitizer is to either define a set
of domains you allow the URL to point to, and to rewrite the URLs to proxy
the request through a 1st party server for privacy concerns

This one.

[otherdaniel]

My 2 cents: We should keep the v1 focused on XSS. I'd really like the sanitizer to be easy to use for not-security-trained web devs.

v2: I think it makes a lot of sense to look at sanitizer-adjacent use-cases for a v2, where an application might have app-specific constraints (like conditions on outgoing urls, or styles, or whatnot). If we can provide additional dev value without impairing the main use case we should probably do so, just not in the first step

[mozfreddyb]

Fully agree here. I'm assigning the v2 milestone, which means it's explicitly marked as interesting, but for later.

[otherdaniel]

Related spec for declarative URL validation, which could apply here: whatwg/urlpattern#26

(Still v2 milestone, just wanted to surface this here so it doesn't get lost.)