-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[BUG] Turn twitter cards off by default to avoid security issues #2536
Comments
Thanks @shankari! I think we can address the security concern by calling If we can address this security flaw and you still want the ability to skip these meta tags I'd rather make them opt-out rather than opt-in, as they're fairly low overhead and they're a nice touch in various places you might post links to your app. |
@alexcjohnson I have a workaround for this now, so I will see if one of my summer interns can work on this when they get started in a month or so. If not, I will see if I can spend a Friday working on it. |
@shankari we believe #2540 will remove any possibility of this vulnerability, however even before that we've been unable to reproduce the problem. In every flavor we've tried |
@alexcjohnson we are also an open source project, so you should be able to clone https://github.com/e-mission/op-admin-dashboard and use the docker-compose files to reproduce. I just re-tried and was able to reproduce with the following steps:
Then, copy-paste An alert pops up (see screenshot below) ![]() To answer your specific questions:
|
pip list | grep dash
belowif frontend related, tell us your Browser, Version and OS
Describe the bug
Deploying a dash app with pages allows attackers to embed code into the webapp. This is a potential security vulnerability since it allows attackers to execute arbitrary code in the context of the dash sandbox.
Concretely, if use_pages is true, dash calls self._pages_meta_tags()
dash/dash/dash.py
Line 968 in a8b3ddb
which always adds the twitter and og meta tags
dash/dash/dash.py
Line 906 in a8b3ddb
The twitter meta tag includes the URL
So if the dash app is involved with a URL that includes a
<script>
tag, the script specified in the tag will be executed.Example URL
This causes our dash app to fail cyber security/pen testing scans.
A workaround is to a custom
index_string
which removes allmeta
tags, but that has the disadvantage of not including any meta tags, even the ones that we might want.A better option would be to make the twitter and og meta tags opt-in; it is not clear that specifying a twitter card is necessary for all deployers.
I am happy to submit a PR (
include_card_configs
property) if that would helpScreenshots
Vulnerability report
Page source includes the embedded script
Example of an embedded alert in firefox
The text was updated successfully, but these errors were encountered: