nccgroup / sobelow

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

(Protocol.UndefinedError) error #142

Open krispetek opened 1 year ago

krispetek commented 1 year ago

I have a Phoenix 1.7.6 project (Elixir 1.15.2/OTP 26) and when I run mix.sobelow I get the following error:

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

** (Protocol.UndefinedError) protocol String.Chars not implemented for {:., [line: 95, column: 67], [{:notification, [line: 95, column: 55], nil}, :content]} of type Tuple
    (elixir 1.15.2) lib/string/chars.ex:3: String.Chars.impl_for!/1
    (elixir 1.15.2) lib/string/chars.ex:22: String.Chars.to_string/1
    (elixir 1.15.2) lib/enum.ex:4055: Enum.join_non_empty_list/3
    (elixir 1.15.2) lib/enum.ex:4340: Enum.join_list/2
    (sobelow 0.12.2) lib/sobelow/finding.ex:60: Sobelow.Finding.normalize/1
    (elixir 1.15.2) lib/enum.ex:1693: Enum."-map/2-lists^map/1-1-"/2
    (sobelow 0.12.2) lib/sobelow/xss/raw.ex:11: Sobelow.XSS.Raw.run/4
    (elixir 1.15.2) lib/enum.ex:984: Enum."-each/2-lists^foreach/1-0-"/2

Is there a way to find out what is causing this error? Atom :notification is mentioned on line 95 but that atom is never used on that line in any of the files I have.

houllette commented 1 year ago

Hmm interesting - there's an open PR right now to bump up support for Elixir 1.15 in Sobelow that may fix this issue. Let me look into this a bit more once we push that through!

houllette commented 1 year ago

Alrighty, the new version of Sobelow supports Elixir 1.15 now - are you still experiencing this issue? I couldn't recreate this issue on my own, so any more context on the offending code would be helpful 🙂

krispetek commented 1 year ago

Yes, I'm still experiencing this issue unfortunately. The error is a bit confusing, as I said, : notification atom is not used on line 95 anywhere in my code so I have no idea what piece of code is causing this error. Can you give me some pointers on what should I look for?

GriffinMB commented 1 year ago

You're looking for something like notification.content, possibly in an eex template. If you could find it, and let us know what it is, it should help us add a case to the finding normalization and fix this bug.

krispetek commented 1 year ago

I found the problem... it's a helper function which creates some HTML outside of templates.

So, this function creates HTML for notification message and takes in notification.count and notification.content["recipient"]}. What's interesting is that if I remove either notification.count or notification.content["recipient"]}` from the HTML, Sobelow works just fine but if both exist, the error shows.

defp notification_message(%Notification{entity_type: "new_message"} = notification) do
    raw("You have <span class=\"text-neutral-800 font-bold\">#{notification.count} new messages</span> from <span class=\"text-neutral-800 font-bold\">#{notification.content["recipient"]}</span>")
end
realcorvus commented 11 months ago

Thanks @krispetek for filing this, I'm seeing the same type of error.

houllette commented 11 months ago

Thanks for the bump, @realcorvus - I have a potential fix in draft locally that I will try to wrap up here shortly, test it and subsequently push it out.

Thanks for the patience all!

houllette commented 8 months ago

A fix for this has been merged to master - please let me know if this has solved the issues you've been encountering! Apologies for the delay!