Skip to content

Commit

Permalink
Prevent String.to_atom/1 calls
Browse files Browse the repository at this point in the history
  • Loading branch information
danschultzer committed Nov 22, 2019
1 parent 41127fa commit 026105e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 4 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## v0.4.4 (TBA)

* [`PowAssent.Plug`] Now uses `String.to_existing_atom/1` in `PowAssent.Plug.providers_for_current_user/1`
* [`PowAssent.Plug`] Fixed security issue by removing `String.to_atom/1` for user provided binary in `PowAssent.Plug.authorize_url/3` and `PowAssent.Plug.callback/4`
* [`PowAssent.Config`] `PowAssent.Config.get_provider_config/2` now accepts binary provider

## v0.4.3 (2019-11-20)

* Removed `:phoenix_html` dependency requirement
Expand Down
16 changes: 14 additions & 2 deletions lib/pow_assent/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,27 @@ defmodule PowAssent.Config do
@doc """
Gets the provider configuration from the provided configuration.
"""
@spec get_provider_config(t(), atom()) :: t() | no_return
@spec get_provider_config(t(), atom() | binary()) :: t() | no_return
def get_provider_config(config, provider) do
config
|> get_providers()
|> get(provider)
|> get_for_provider(provider)
|> Kernel.||(raise_error("No provider configuration available for #{provider}."))
|> add_global_config(config)
end

defp get_for_provider(providers_config, provider) when is_atom(provider) do
get(providers_config, provider)
end
defp get_for_provider(providers_config, provider) when is_binary(provider) do
Enum.find_value(providers_config, fn {key, value} ->
case Atom.to_string(key) do
^provider -> value
_any -> false
end
end)
end

defp add_global_config(provider_config, config) do
[
:http_adapter,
Expand Down
3 changes: 1 addition & 2 deletions lib/pow_assent/plug.ex
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ defmodule PowAssent.Plug do
conn
|> Pow.Plug.current_user()
|> get_all_providers_for_user(config)
|> Enum.map(&String.to_atom(&1.provider))
|> Enum.map(&String.to_existing_atom(&1.provider))
end

defp get_all_providers_for_user(nil, _config), do: []
Expand Down Expand Up @@ -232,7 +232,6 @@ defmodule PowAssent.Plug do
|> get_provider_config(provider, redirect_uri)
end
defp get_provider_config(config, provider, redirect_uri) do
provider = String.to_atom(provider)
config = Config.get_provider_config(config, provider)
strategy = config[:strategy]
provider_config =
Expand Down
10 changes: 10 additions & 0 deletions test/pow_assent/config_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,14 @@ defmodule PowAssent.ConfigTest do
assert Config.get_provider_config([http_adapter: HTTPAdapater, json_adapter: JSONAdapter, jwt_adapter: JWTAdapter], :provider1) ==
[http_adapter: HTTPAdapater, json_adapter: JSONAdapter, jwt_adapter: JWTAdapter, a: 1]
end

test "get_provider_config/2 with binary provider" do
config = [providers: [provider1: [a: 1], provider2: [b: 2]]]

assert Config.get_provider_config(config, "provider1") == [a: 1]

assert_raise PowAssent.Config.ConfigError, "No provider configuration available for non_existent.", fn ->
refute Config.get_provider_config(config, "non_existent")
end
end
end

0 comments on commit 026105e

Please sign in to comment.