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

XSS/Privacy protection URL Whitelist for external images by CSP #738

Open
eternal-flame-AD opened this issue Nov 18, 2024 · 2 comments
Open
Labels
a:feature New feature or request in:server

Comments

@eternal-flame-AD
Copy link
Member

Is your feature request related to a problem? Please describe.

It would be a strong protection against things like this:

GHSA-xv6x-456v-24xh

GHSA-3244-8mff-w398

it would also be useful in cases like you want to see images in a message but not really 100% trust there can never be bad content (an example is if you receive webhook, the sender might not have properly sanitized the markdown)

Describe the solution you'd like

A config or admin option to whitelist which URLs can be rendered. On the WebUI we serve a CSP header to prevent images not in the whitelist from being updated. Something like (untested):

Content-Security-Policy: default-src 'self'; img-src 'self' data: https://my.images.net/; media-src 'none'; script-src: https://gotify/static/js/; style-src: https://gotify/static/css/; style-src-attr 'self' 'unsafe-inline';

On the Android client we will probably need to implement the same algorithm: https://www.w3.org/TR/CSP/#match-url-to-source-expression

Describe alternatives you've considered

An option to globally disable all remote images (will need to rely on the markdown renderer's correctness).

Additional context

The logic of interpolating %CONFIG% when serving the UI at runtime may need to be refactored. The general idea is to precompute the script content, hash it and write it in the CSP header.

@eternal-flame-AD eternal-flame-AD added a:feature New feature or request in:server labels Nov 18, 2024
@jmattheis
Copy link
Member

This should already be configurable via additional headers, at least for the browser part. I'm not sure if I'd want to change the default here, as this likely breaks some setups.

Do you think this should be a native feature, or would a hardening guide on the website be enough?

@eternal-flame-AD
Copy link
Member Author

eternal-flame-AD commented Nov 19, 2024

The reason is I want it to be in the server is to make it consistent on all clients: we cannot expect every client (including our own Android version) to fully parse any valid CSP (the specs are huge and I don't think it is a good idea to loosely parse a CSP added via a reverse proxy, may break things or cause bypass). I think it would be good if we can just take the subset (URL expressions) which would work across all clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature New feature or request in:server
Development

No branches or pull requests

2 participants