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

CSP with commas are parsed in a confusing way #10

Open
briansmith opened this issue Apr 24, 2019 · 2 comments · May be fixed by #64
Open

CSP with commas are parsed in a confusing way #10

briansmith opened this issue Apr 24, 2019 · 2 comments · May be fixed by #64

Comments

@briansmith
Copy link

briansmith commented Apr 24, 2019

Consider the input:

base-uri 'none';
default-src 'self';
object-src 'none';
style-src 'unsafe-inline';
script-src 'unsafe-inline' 'sha256-HEtTzbIgu0I33A3DZbJTheKFftQg+kS2n0OFuHExFuc='

That passes the evaluator just fine, with no warnings.

Now, consider this stronger combination of two policies:

base-uri 'none';
default-src 'self';
object-src 'none';
style-src 'unsafe-inline',
script-src 'unsafe-inline' 'sha256-HEtTzbIgu0I33A3DZbJTheKFftQg+kS2n0OFuHExFuc='

The intent of this second input is to require that a script be loaded from self AND match the given hash, if the browser supports CSP hash, by asking for the intersection of two policies. This is a stronger policy than the original policy. However, the evaluator complains that "'self' can be problematic if you host JSONP, Angular or user uploaded files" because it doesn't notice the script-src. It also complains about the style-src directive because it doesn't recognize the comma that separates the two policies.

Ideally, the evaluator should be extended to understand multiple policies joined using ,.

This example uses CSP hash, which is rare. However, I believe several people have advocated for a similar technique of combining multiple policies that uses CSP nonce instead of CSP hash, so it would be good to support this pattern.

@lweichselbaum
Copy link
Member

Thank you for reporting Brian.
We're aware of the issue (we actually do use and recommend double policies [1]), but we currently don't have the bandwidth to implement support for multiple policies in the CSP Evaluator.
This would require intersecting findings across policies which would need a complex rule set for matching hosts/paths/wildcards/protocols/etc.
E.g. Findings for entries like https://www.google.com/, *.google.com/, https://www.google.com/bar and https:// would need to be matched across policies and potentially across different directives (default-src/script-src) to determine, if the intersection would still allow a CSP bypass.

For simple cases like the one you posted above, you can evaluate policies individually and manually intersect results.

[1] https://speakerdeck.com/lweichselbaum/csp-a-successful-mess-between-hardening-and-mitigation?slide=44

@briansmith
Copy link
Author

There are multiple aspects of this, some of which are easier to fix than others:

  • If I understand my own bug report correctly, the evaluator doesn't parse these policies correctly. At the very least, I think the parser could/should be fixed.
  • If the evaluator were to split policies on commas properly during parsing, then it could process each policy independently and report results for each independently.
  • If the evaluator were to process policies split on commas properly, then it could implement "If any of the policies would have generated a good result, then the entire combined policy generates a good result. Otherwise, tell the user that multiple policies combined using commas are not fully supported."

@MaxNad MaxNad linked a pull request Jan 8, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants