Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Sanitize href props with xss vulnerability #999

Closed
wants to merge 1 commit into from

Conversation

AnnMarieW
Copy link
Contributor

Sanitize html props that are vulnerable to xss vulnerability if user data is inserted.

Here's an example:


from dash import Dash, html
import dash_bootstrap_components as dbc

app=Dash(__name__)

app.layout = html.Div(
    dbc.NavLink("link",  href='javascript:alert(1)')
)

if __name__ == '__main__':
    app.run_server(debug=True)

Applied the same fix as plotly/dash#2732

The href prop is sanitized using the braintree/sanitize-url

Content with vulnerabilities are replaced with "about:blank", and an error message is sent.

Dash 2.15 has a new prop:

Add special key _dash_error to setProps, allowing component developers to send error without throwing in render. Usage props.setProps({_dash_error: new Error("custom error")})

I've included this, but it only sends the error message when using Dash 2.15. Earlier Dash versions will not show an error in the console or the dev tools, but the prop is still sanitized.

Would you like to bump the install_requires to dash>=2.15 ? I'm not sure how that would affect people that need to specify a dash version to get certain flask version.

Before writing the tests would you like to take a quick look? You can run usage.py to see all the components that are updated so far.

To Do:

  • Carousel
  • Tests
  • Missing any other components?

Copy link
Collaborator

@tcbegley tcbegley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

  • Can you add package-lock.json. The mismatched dependencies in package.json is causing CI to fail
  • Requiring Dash 2.15 feels aggressive. If things still work in older versions but we're missing a dev feature like better error messages then that's ok
  • In addition to unit tests, can we write integration test that checks the error is raised?
  • the code is a bit repetitive, do you think we could maybe refactor into a custom hook in src/private? If it works we could even upstream into @plotly/dash-component-plugins then just import it!

@AnnMarieW
Copy link
Contributor Author

Closed in favor of #1000

@AnnMarieW AnnMarieW closed this Feb 3, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants