diff --git a/lib/credo/check/warning/forbidden_function.ex b/lib/credo/check/warning/forbidden_function.ex new file mode 100644 index 000000000..a945309c1 --- /dev/null +++ b/lib/credo/check/warning/forbidden_function.ex @@ -0,0 +1,103 @@ +defmodule Credo.Check.Warning.ForbiddenFunction do + use Credo.Check, + id: "EX5030", + base_priority: :high, + category: :warning, + param_defaults: [ + functions: [] + ], + explanations: [ + check: """ + Some functions may be hazardous if used directly. Use this check to + forbid specific functions from being called directly by your application, + while allowing other functions in the same module. + + This check is similar to `Credo.Check.Warning.ForbiddenModule`, but for + specific functions within a module rather than the entire module. + + For example, `:erlang.binary_to_term/1` is vulnerable to arbitrary code + execution exploits when deserializing untrusted data; you may want to point + developers to `Plug.Crypto.non_executable_binary_to_term/2` instead, which + disallows anonymous functions in the deserialized term. + """, + params: [ + functions: """ + List of `{module, function, error_message}` tuples specifying forbidden functions. + + Example: + + functions: [ + {:erlang, :binary_to_term, "Use Plug.Crypto.non_executable_binary_to_term/2 instead."} + ] + """ + ] + ] + + @doc false + @impl Credo.Check + def run(%SourceFile{} = source_file, params \\ []) do + case Params.get(params, :functions, __MODULE__) do + [_ | _] = functions -> + issue_meta = IssueMeta.for(source_file, params) + forbidden_map = build_forbidden_map(functions) + Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, forbidden_map)) + + [] -> + [] + end + end + + defp build_forbidden_map(functions) do + Map.new(functions, fn {module, fun, message} -> + if not is_atom(module), do: raise("Module name must be an atom; got #{inspect(module)}") + if not is_atom(fun), do: raise("Function name must be an atom; got #{inspect(fun)}") + {{module, fun}, to_string(message)} + end) + end + + # Handle calls to erlang modules like :erlang.binary_to_term(x) + defp traverse({{:., meta, [module, function]}, _, _} = ast, issues, issue_meta, forbidden_map) + when is_atom(module) and is_atom(function) do + {ast, append_issue_if_forbidden({module, function}, forbidden_map, issues, issue_meta, meta)} + end + + # Handle calls to Elixir modules like MyModule.my_function(...) + defp traverse( + {{:., meta, [{:__aliases__, _, module_parts}, function]}, _call_meta, _args} = ast, + issues, + issue_meta, + forbidden_map + ) + when is_atom(function) and is_list(module_parts) do + issues = + if Enum.all?(module_parts, &is_atom/1) do + module = Credo.Code.Name.full(module_parts) + append_issue_if_forbidden({module, function}, forbidden_map, issues, issue_meta, meta) + else + issues + end + + {ast, issues} + end + + defp traverse(ast, issues, _issue_meta, _forbidden_map), do: {ast, issues} + + defp append_issue_if_forbidden(mod_fun, forbidden_map, issues, issue_meta, meta) + when is_map_key(forbidden_map, mod_fun) do + message = Map.get(forbidden_map, mod_fun) + [create_issue(issue_meta, meta[:line], mod_fun, message) | issues] + end + + defp append_issue_if_forbidden(_mod_fun, _forbidden_map, issues, _issue_meta, _meta), do: issues + + defp create_issue(issue_meta, line_no, {mod, fun}, message) do + trigger = "#{inspect(mod)}.#{to_string(fun)}" + + format_issue( + issue_meta, + message: "#{trigger} is forbidden: #{message}", + trigger: trigger, + line_no: line_no + ) + end +end diff --git a/test/credo/check/warning/forbidden_function_test.exs b/test/credo/check/warning/forbidden_function_test.exs new file mode 100644 index 000000000..0c7ea21d1 --- /dev/null +++ b/test/credo/check/warning/forbidden_function_test.exs @@ -0,0 +1,165 @@ +defmodule Credo.Check.Warning.ForbiddenFunctionTest do + use Credo.Test.Case + + @described_check Credo.Check.Warning.ForbiddenFunction + + @binary_to_term_error "Use Plug.Crypto.non_executable_binary_to_term/2 instead." + + @erlang_binary_to_term_config [ + functions: [ + {:erlang, :binary_to_term, @binary_to_term_error} + ] + ] + + test "produces no issues when no functions configured" do + """ + defmodule MyModule do + def decode(data) do + :erlang.binary_to_term(data) + end + end + """ + |> to_source_file() + |> run_check(@described_check, functions: []) + |> refute_issues() + end + + describe "Erlang function calls" do + test "will alert on any arity" do + for function_args <- ["", "foo", "foo, bar"] do + """ + defmodule MyModule do + def decode(data) do + :erlang.binary_to_term(#{function_args}) + end + end + """ + |> to_source_file() + |> run_check(@described_check, @erlang_binary_to_term_config) + |> assert_issue(fn issue -> + assert issue.trigger == ":erlang.binary_to_term" + assert issue.message == ":erlang.binary_to_term is forbidden: #{@binary_to_term_error}" + end) + end + end + + test "ignores other Erlang functions" do + """ + defmodule MyModule do + def my_function do + :erlang.term_to_binary(%{foo: "bar"}) + end + end + """ + |> to_source_file() + |> run_check(@described_check, @erlang_binary_to_term_config) + |> refute_issues() + end + end + + describe "Elixir function calls" do + test "will alert on any arity" do + error_message = "This function is dangerous." + + for function_args <- ["", "foo", "foo, bar"] do + """ + defmodule MyModule do + def dangerous do + SomeModule.dangerous_function(#{function_args}) + end + end + """ + |> to_source_file() + |> run_check(@described_check, + functions: [ + {SomeModule, :dangerous_function, error_message} + ] + ) + |> assert_issue(fn issue -> + assert issue.trigger == "SomeModule.dangerous_function" + assert issue.message == "SomeModule.dangerous_function is forbidden: #{error_message}" + end) + end + end + + test "handles nested module names" do + custom_error = "Don't use this." + + """ + defmodule MyModule do + def call do + Some.Nested.Module.forbidden_func() + end + end + """ + |> to_source_file() + |> run_check(@described_check, + functions: [ + {Some.Nested.Module, :forbidden_func, custom_error} + ] + ) + |> assert_issue(fn issue -> + assert issue.trigger == "Some.Nested.Module.forbidden_func" + assert issue.message == "Some.Nested.Module.forbidden_func is forbidden: #{custom_error}" + end) + end + + test "allows non-forbidden functions from the same module" do + """ + defmodule MyModule do + def safe do + SomeModule.safe_function(foo) + end + end + """ + |> to_source_file() + |> run_check(@described_check, + functions: [ + {SomeModule, :dangerous_function, "This function is dangerous."} + ] + ) + |> refute_issues() + end + end + + test "detects multiple violations" do + """ + defmodule MyModule do + def decode(data) do + :erlang.binary_to_term(data) + end + + def other do + SomeModule.dangerous_function(:arg) + end + end + """ + |> to_source_file() + |> run_check(@described_check, + functions: [ + {:erlang, :binary_to_term, "Use safe alternative."}, + {SomeModule, :dangerous_function, "This is dangerous."} + ] + ) + |> assert_issues(fn issues -> + messages = Enum.map(issues, & &1.message) |> Enum.sort() + assert [":erlang.binary_to_term" <> _, "SomeModule.dangerous_function" <> _] = messages + end) + end + + test "handles piped calls" do + """ + defmodule MyModule do + def decode(data) do + data + |> Base.decode64!() + |> :erlang.binary_to_term() + |> IO.inspect() + end + end + """ + |> to_source_file() + |> run_check(@described_check, @erlang_binary_to_term_config) + |> assert_issue() + end +end