nccgroup / sobelow

Security-focused static analysis for the Phoenix Framework
Apache License 2.0
1.66k stars 92 forks source link

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

Closed realcorvus closed 1 year ago

realcorvus commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

houllette commented 1 year ago

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 commented 1 year ago

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.