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.

Sign up for GitHub

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

Jump to bottom

Update: Cross Site Scripting Prevention Cheat Sheet #1480

Closed
dp-anto opened this issue Sep 5, 2024 · 4 comments · Fixed by #1484
Closed

Update: Cross Site Scripting Prevention Cheat Sheet #1480

dp-anto opened this issue Sep 5, 2024 · 4 comments · Fixed by #1484
Labels
ACK_OBTAINED Issue acknowledged from core team so work can be done to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet.

Comments

@dp-anto
Copy link
Contributor

dp-anto commented Sep 5, 2024 •
edited
Loading

What is missing or needs to be updated?

Under the section Safe Sinks it is stated:

For a comprehensive list, check out the DOMPurify allowlist

Opening the link, a broad list of attributes is presented and between them there are also the src and href attributes which are famous for accepting the javascript: directive which allows to execute any arbitrary JS code.

How should this be resolved?

I think more info should be provided. Maybe is it just the link wrong? Or did I misunderstand anything?
@dp-anto dp-anto added ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet. labels Sep 5, 2024
@jmanico
Copy link
Member

jmanico commented Sep 6, 2024

I think this is a fair concern. The src, href and other attributes that take a URL can be dangerous. DomPurify filters out the URL for these attributes to make sure they are only safe protocols.

Would you care to give us a PR clarifying this?

@mackowski mackowski added ACK_OBTAINED Issue acknowledged from core team so work can be done to fix it. and removed ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. labels Sep 9, 2024
@dp-anto
Copy link
Contributor Author

dp-anto commented Sep 9, 2024 •
edited
Loading

To be honest I don't know how to better explain the section.

Maybe just removing the sentence For a comprehensive list, check out the DOMPurify allowlist? At least this would not lead the reader to think that each attribute in the list is a secure one.

dp-anto added a commit to dp-anto/CheatSheetSeries that referenced this issue Sep 9, 2024
@dp-anto
Update Cross_Site_Scripting_Prevention_Cheat_Sheet.md
150b08c 
OWASP#1480
@dp-anto dp-anto mentioned this issue Sep 9, 2024
Merged
8 tasks
@dp-anto
Copy link
Contributor Author

dp-anto commented Sep 9, 2024

Anyway, I've just create a pull request with my proposal #1484

@jmanico
Copy link
Member

jmanico commented Sep 9, 2024

I made a small request for a change in your PR proposal, but otherwise it looks good

@szh szh linked a pull request Sep 9, 2024 that will close this issue
Update Cross_Site_Scripting_Prevention_Cheat_Sheet.md #1484
Merged
8 tasks
szh pushed a commit that referenced this issue Sep 9, 2024
@dp-anto
Update Cross_Site_Scripting_Prevention_Cheat_Sheet.md (#1484)
46d3fe3 
* Update Cross_Site_Scripting_Prevention_Cheat_Sheet.md

#1480

* Update Cross_Site_Scripting_Prevention_Cheat_Sheet.md
@szh szh closed this as completed in #1484 Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ACK_OBTAINED Issue acknowledged from core team so work can be done to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update Cross_Site_Scripting_Prevention_Cheat_Sheet.md dp-anto/CheatSheetSeries
3 participants
@jmanico @mackowski @dp-anto