Issues with reflected-xss (was Re: CfC: Transition CSP2 to CR.)

Mike West <mkwst@google.com> wrote:
> Brian Smith <brian@briansmith.org> wrote:
>> As for reflected-xss, I agree with deferring it to CSP3. Even if the
>> working group ultimately decides that CSP shouldn't be a purely
>> restrictive mechanism, there are several issues with how reflected-xss
>> is specified (and underspecified) in the CSP2 draft. (I think I have a
>> rough list of issues with reflected-xss that need to be addressed, if
>> somebody wants them. But, I don't have time to write it up in the same
>> level of detail I usually provide.)
>
> A rough list would be great. I really appreciate the time you've spent on
> detailed feedback, but I don't want to ask you to dive into every detail on
> this as well.
>
> It's marked as "at risk", which gives us freedom to modify/remove during CR.
> Given that Firefox won't implement anyway (as there's no xss auditor to turn
> on), it's more or less up to Apple and Microsoft to determine whether they'd
> like to implement this directive.

Here are my notes from a few months ago. I tried to clean them up a
bit, but they are still really rough. Sorry in advance.

Also, I've attached a Python program I wrote at that time for testing
the interaction of XSS auditor policies specified in 0 or more
reflected-xss CSP directives and 0 or more X-XSS-Protection header
fields. I've forgotten how to use it, but apparently you can
experiment with it based on this example:

http://localhost:8000/?reflected-xss=allow&script=%3Cscript%3Ealert(1)%3C/script%3E&X-XSS-Protection=0

http://localhost:8000/?reflected-xss=allow&reflected-xss=filter&script=%3Cscript%3Ealert(1)%3C/script%3E&X-XSS-Protection=0

* It hasn't even been decided in WHATWG or W3C whether web browsers
should do XSS filtering.

* The semantics of the feature are completely unspecified. The feature
says "block" means the browser should do something unspecified and
"filter" means it should do something unspecified. Presumably, "allow"
means "conform to the HTML standard," but that isn't even stated. Note
that it seems very difficult to accurately and completely specify the
behavior of the XSS auditors in shipping browsers.

* Even if reflected-xss is added to CSP 2, servers will need to add
X-XSS-Protection to responses for many years to support current and
older browsers. And, thus, for a long time, the CSP directives will be
completely redundant with X-XSS-Protection.

* Now that referrer is no longer in CSP 2, reflected-xss is the only
directive that requires special rules for combining multiple policies.
In particular, the spec doesn't specify what happens when the
documents has multiple policies that conflict with each other, such as
"Content-Security-Policy: reflected-xss allow, reflected-xss block,
reflected-xss filter". This special case and extra complexity is
counterproductive for CSP.

* Any reasonable implementation of "block" and "filter" would result
in a non-conformant HTML implementation, AFAICT, because the HTML
specification seems to only consider the "allow" case.

* There is no default specified. The HTML specification is written as
though the default is "allow" but browsers that implement XSS filters
implement different defaults.

* I, and a lot of other people, think that "filter" is harmful or at
least questionable. A browser shouldn't be required to implement
"filter." So, I think, at a minimum, "filter" should be dropped.

* If a browser implements only "allow" and "block," it isn't specified
what it should do when it encounters "reflected-xss filter."

* The interaction between reflected-xss and X-XSS-Protection isn't
specified. In particular, what happens when the reflected-xss
directive values conflict with conflict with the directives given in
X-XSS-Protection?

* X-XSS-Protection isn't stated to be deprecated in the CSP2 spec.

* The main advantage of reflected-xss over X-XSS-Protection is that
reflected-xss allows for reporting, whereas X-XSS-Protection doesn't
have reporting. This should be called out.

* CSP source list directives have their keywords-like syntax elements
quoted like 'none', 'self', but reflected-xss doesn't require/allow
quotes of its similar syntax elements. (This also applies to sandbox.)
I think that at least this should be called out, as it seems likely to
trip people up. (I may be biased here. I've written a program to test
how reflected-xss works, and I made the mistake of assuming that the
directive values were quoted in my initial iteration.)

* It seems unnecessary to prohibit this directive from occurring
within <meta>, but it is currently specified to be prohibited from
occuring within <meta>.

Cheers,
Brian