-
-
Notifications
You must be signed in to change notification settings - Fork 644
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
Comments
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? |
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. |
Okay, understood, can be added to gotify. Do you know how other apps with user generated content handle this? It would be cool to have a client independent solution. |
I am thinking about two ways to do this, one is a |
Specifically about image injection, is this really a problem we have to solve? I think if someone can inject images to a message, then getting tracked via a remote http call seems not that important or the much lesser problem than the image injection. |
It will probably highly depend on cases. A random webhook service is unlikely to sanitize webhook content for you (which may not be attributed to or endorsed by the service itself). My original thought was trusting whoever generated the webhook does not mean there can be no 3rd party influence on the webhook content. (For example if you have a webhook that shows updates to a social media or RSS, it is nice to have these formatted in a pretty way but does not allow arbitrary remote content) Having a URL whitelist is a good way to still allow Markdown rendering without any leaked calls IMO. Another way to not do this in the main server is the jq plugin I mentioned where I can provide an addon function to sanitize the webhooks, but it is still a limitation that one plugin can only act as one app. |
I think a user friendly implementation would be that all content is blocked by default and we have a banner on each message (similar to emails) with load remote content. Or "always load remote content from github.io/coolimages". Maybe it's even for a specific application. I'd expect that this setting must be on the user, as different user likely allow different urls. Tho, as maintainer I feel like this is really overkill for a feature most user don't use. So maybe we have just the "load remote content for this message" button and a user setting with "always load remote content for application x" (so that we can add this boolean to the application) What do you think? |
I agree with your evaluation although I am not sure exactly what is a typical deployment of gotify, my assumption is most deployment is highly homogeneous (such as within a department delivering IT alerts) and they may even prefer a policy that is global, correct (browser built-in CSP as opposed to custom logic) and not easily reconfigurable on the user level. I am not in a rush for this to be upstream, just a suggestion and we can see if more people want this and the exact use case |
Okay, then let's wait for some feedback. |
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.The text was updated successfully, but these errors were encountered: