3 ms·

Interesting bug. It makes me appreciate working with languages that support pattern matching like Elixir. I think this type of bug would have been preventable

by nickjj 7y ago

Interesting bug. It makes me appreciate working with languages that support pattern matching like Elixir. I think this type of bug would have been preventable with pattern matching because that single controller function would have likely been split out into 2 functions that each matched against a specific HTTP method, and when the unmatched pattern came into play (the HEAD request) it would have failed to find a match and then threw an error instead of processing it as something else.

griffinmb 7y ago
This class of bug (CSRF bypass via route confusion) is probably more common in Phoenix apps. I’ve found a handful of apps vulnerable to this issue with Sobelow. People create (for example) a get ‘/profile’ and a post ‘/profile’, and the action intended to correspond with post requests really just pattern matches against params. I’ve also seen at least one app implement this properly, matching against the HTTP method as you described.
- nickjj 7y ago
  To be safe from this in Phoenix that would look like this right? def profile(conn = %{method: "GET"}, params) do # ... end def profile(conn = %{method: "POST"}, %{"user" => user_params) do # ... end This would be in a case where your router looks like: get "/profile", UserController, :profile, as: :user post "/profile", UserController, :profile, as: :user That's what I'm doing in my code base at the moment. Mainly thanks to Changelog open sourcing their platform, and you can see that pattern being used here: https://github.com/thechangelog/changelog.com/blob/f9b0a758746f2cec10781b77eb0541cce765e054/lib/changelog_web/controllers/person_controller.ex#L90 https://github.com/thechangelog/changelog.com/blob/f9b0a7587... The above seems like the natural way to do it with Phoenix once you get a hang of pattern matching.
  - griffinmb 7y ago
    Yep, that’s the way!

by nickjj 7y ago

griffinmb 7y ago

This class of bug (CSRF bypass via route confusion) is probably more common in Phoenix apps. I’ve found a handful of apps vulnerable to this issue with Sobelow. People create (for example) a get ‘/profile’ and a post ‘/profile’, and the action intended to correspond with post requests really just pattern matches against params. I’ve also seen at least one app implement this properly, matching against the HTTP method as you described.

nickjj 7y ago
To be safe from this in Phoenix that would look like this right? def profile(conn = %{method: "GET"}, params) do # ... end def profile(conn = %{method: "POST"}, %{"user" => user_params) do # ... end This would be in a case where your router looks like: get "/profile", UserController, :profile, as: :user post "/profile", UserController, :profile, as: :user That's what I'm doing in my code base at the moment. Mainly thanks to Changelog open sourcing their platform, and you can see that pattern being used here: https://github.com/thechangelog/changelog.com/blob/f9b0a758746f2cec10781b77eb0541cce765e054/lib/changelog_web/controllers/person_controller.ex#L90 https://github.com/thechangelog/changelog.com/blob/f9b0a7587... The above seems like the natural way to do it with Phoenix once you get a hang of pattern matching.
- griffinmb 7y ago
  Yep, that’s the way!