Skip to content

Add warning for html.script #924

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

Closed
Archmonger opened this issue Feb 13, 2023 · 12 comments · Fixed by #970
Closed

Add warning for html.script #924

Archmonger opened this issue Feb 13, 2023 · 12 comments · Fixed by #970
Labels
flag-good-first-issue A well defined and self-contained task. priority-3-low May be resolved one any timeline. type-docs About changes and updates to documentation

Comments

@Archmonger
Copy link
Contributor

Archmonger commented Feb 13, 2023

Current Situation

Currently, there is no documented warnings for the potential of XSS attacks when using html.script

Proposed Actions

We should add a disclaimer to warn users not to use raw user inputs (from any untrusted data source) within the script contents to avoid XSS attacks.

@Archmonger Archmonger added the type-docs About changes and updates to documentation label Feb 13, 2023
@Archmonger Archmonger added this to the Luxury milestone Feb 13, 2023
@Archmonger Archmonger added flag-good-first-issue A well defined and self-contained task. priority-3-low May be resolved one any timeline. labels Feb 13, 2023
@rmorshea rmorshea removed this from the Luxury milestone Feb 21, 2023
@ZEUS-03
Copy link
Contributor

ZEUS-03 commented Apr 17, 2023

Can you assign me the task? Will be a good starting point.

@Archmonger
Copy link
Contributor Author

We don't utilize the issue assignment feature on GitHub. The first person to open a draft PR gets priority to resolve it.

@ZEUS-03
Copy link
Contributor

ZEUS-03 commented Apr 17, 2023

ok thanks! will do that.

@rmorshea
Copy link
Collaborator

@Archmonger, was your intention for this to be in the docstring.

@Archmonger
Copy link
Contributor Author

Yeah, since the docstring gets turned into auto-docs.

@ZEUS-03
Copy link
Contributor

ZEUS-03 commented Apr 17, 2023

hey! can you please tell me where do i have to put or write the documention i mean in which file?

@rmorshea
Copy link
Collaborator

https://github.com/reactive-python/reactpy/blob/main/src/reactpy/html.py

@rmorshea
Copy link
Collaborator

@Archmonger, can we create a follow-up ticket for a tool that would sanitize inputs to scripts? I think we can do more to help mitigate this risk for users.

@Archmonger
Copy link
Contributor Author

You think this could be added to our flake8 plugin?

@rmorshea
Copy link
Collaborator

rmorshea commented Apr 23, 2023

It's possible, but if that's all we supplied users might improperly sanitize inputs. Better to provide a standardized solution.

@Archmonger
Copy link
Contributor Author

Archmonger commented Apr 23, 2023

That might be a bit over engineered for a tag that we shouldn't incentivize using.

In my opinion, due to script tags being inherently dangerous we should treat them similar to React dangerouslySetInnerHtml.

It's best to get the idea into user's heads that "you can use this but it's risky so you probably shouldn't."

@rmorshea
Copy link
Collaborator

I like that idea. We could require the script to be passed as a kwarg with a scary name like insecurely_eval or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag-good-first-issue A well defined and self-contained task. priority-3-low May be resolved one any timeline. type-docs About changes and updates to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants