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.

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

Encoded "script" Elements don't get scrubbed #64

Open
PJUllrich opened this issue Dec 12, 2023 · 4 comments
Open

Encoded "script" Elements don't get scrubbed #64

PJUllrich opened this issue Dec 12, 2023 · 4 comments

Comments

@PJUllrich
Copy link

PJUllrich commented Dec 12, 2023

Hello there,

first of all, thank you very much for this super useful library ❤️🙏

I'm running into an "issue" (I'm unsure whether it's an issue or intended behaviour) that some elements don't get scrubbed if their < and > signs are encoded. For example:

# Works as expected
iex(1)> HtmlSanitizeEx.html5("<script>alert('xss');</script>")
"alert('xss');"

# Doesn't work as expected (the "script" tags aren't removed)
iex(2)> HtmlSanitizeEx.html5("&lt;script&gt;alert('xss');&lt;/script&gt;")
"&lt;script&gt;alert('xss');&lt;/script&gt;"

If I render the second string in my html with raw(@safe_content), it becomes <script>alert('xss');</script> again.

Now, I'm unsure about the implications of this. In my case, the string is user input and I render the content as described in my HEEX template because it can contain code snippets. What do you think? Is there a possible vulnerability here or does everything work as intended? :)

@rrrene
Copy link
Owner

rrrene commented Dec 12, 2023

Is there a possible vulnerability here or does everything work as intended? :)

Those are not necessarily mutually exclusive. But I have to be impolite and ask a question in return:

If I render the second string in my html with raw(@safe_content), it becomes <script>alert('xss');</script> again.

Is this really what Phoenix should do? I will have to check ...

@PJUllrich
Copy link
Author

I'm not sure either, but what I can do is simply replace the &lt; and &gt; values with < and > again and then run the sanitizer. That should take care of that problem.

@rrrene
Copy link
Owner

rrrene commented Dec 14, 2023

@PJUllrich So, I did a quick check and the result is not consistent with I understood from your description:

image

Am I testing the "right" raw/1 function?

@PJUllrich
Copy link
Author

@rrrene I DMed you on Twitter :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants