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

Add support for HEEx to Sobelow.XSS.Raw #123

Merged
merged 2 commits into from Mar 27, 2023
Merged

Add support for HEEx to Sobelow.XSS.Raw #123

merged 2 commits into from Mar 27, 2023

Conversation

realcorvus
Copy link
Contributor

Today, if you're using HEEx for templates in Phoenix, and have an XSS vulnerability, Sobelow will not detect it. Tested in https://github.com/securityelixir/potion_shop

@ potion_shop % mix sobelow -i SQL,Config

... SCAN COMPLETE ...

@ potion_shop % 

This branch:

@ potion_shop % mix sobelow -i SQL,Config

XSS.Raw: XSS - Low Confidence
File: lib/carafe_web/templates/potion/show.html.heex
Line: 19
Variable: review

-----------------------------------------------

... SCAN COMPLETE ...

@ potion_shop % 

When testing this PR, I ran into a problem where this branch (master) does not run locally, it just prints the current version of Sobelow and exits. To get it running again, go to lib/mix/tasks/sobelow.ex, line 185 and comment out:

+      #!is_nil(version) ->
+      #  Sobelow.version()

@houllette
Copy link
Collaborator

Nice add, @realcorvus! Will review ASAP and merge in.

I ran into a problem where this branch (master) does not run locally, it just prints the current version of Sobelow and exits.

Good catch, working on a hot-fix now - I'm surprised mix tests didn't pick it up when I threw it in. Another reason why more robust testing is on my immediate roadmap.

@realcorvus
Copy link
Contributor Author

I just realized there's an issue with the String.replace logic:

String.replace(template_path, "eex", "heex")

iex(1)> template_path = "/Users/hi/lib/test/veryeextreme/show.html.eex"
"/Users/hi/lib/test/veryeextreme/show.html.eex"
iex(2)> String.replace(template_path, "eex", "heex")
"/Users/hi/lib/test/veryheextreme/show.html.heex"
iex(3)> 

The proper way to do this is by the last three characters of the template_path, will update now.

@realcorvus realcorvus closed this Mar 27, 2023
@realcorvus realcorvus reopened this Mar 27, 2023
@houllette houllette mentioned this pull request Mar 27, 2023
@houllette
Copy link
Collaborator

I had a chance to review and it's looking pretty good!

Something else I figured I should bring up before merging this in - should we be considering the implications of LiveEEx template files (.leex) as well as .heex as part of this PR?

@houllette houllette mentioned this pull request Mar 27, 2023
@houllette
Copy link
Collaborator

After some external discussion, support for .leex templates will be introduced at a later point in time as a more concerted effort to better support LiveView in Sobelow.

@houllette houllette merged commit 4f412df into nccgroup:master Mar 27, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants