From 9783a525d9c493b56239ae22681cd1d0c1690476 Mon Sep 17 00:00:00 2001 From: Tyler Young Date: Sat, 7 Feb 2026 07:06:08 -0600 Subject: [PATCH 1/3] Add check for forbidden functions This complements the existing `Credo.Check.Warning.ForbiddenModule`. In our codebase, there are a number of specific functions from otherwise mundane modules that we'd like to forbid. For instance, `:erlang.binary_to_term/{1,2}` is one we found being used in a surprisingly (scarily) large number of places. This functionality could potentially be included as an expansion of `ForbiddenModule`, but it would actually make more sense to me to go the other way and allow ForbiddenFunction to be configured to warn on all functions in a given module. --- lib/credo/check/warning/forbidden_function.ex | 91 ++++++++++ .../check/warning/forbidden_function_test.exs | 165 ++++++++++++++++++ 2 files changed, 256 insertions(+) create mode 100644 lib/credo/check/warning/forbidden_function.ex create mode 100644 test/credo/check/warning/forbidden_function_test.exs diff --git a/lib/credo/check/warning/forbidden_function.ex b/lib/credo/check/warning/forbidden_function.ex new file mode 100644 index 000000000..8f245c875 --- /dev/null +++ b/lib/credo/check/warning/forbidden_function.ex @@ -0,0 +1,91 @@ +defmodule Credo.Check.Warning.ForbiddenFunction do + use Credo.Check, + 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 + issue_meta = IssueMeta.for(source_file, params) + functions = Params.get(params, :functions, __MODULE__) + forbidden_map = build_forbidden_map(functions) + + Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, forbidden_map)) + 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 + module = Module.concat(module_parts) + {ast, append_issue_if_forbidden({module, function}, forbidden_map, issues, issue_meta, meta)} + 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 From bb46fbdbf26069bc9d9a4d3952892637311582fb Mon Sep 17 00:00:00 2001 From: Tyler Young Date: Sat, 7 Feb 2026 07:10:39 -0600 Subject: [PATCH 2/3] Don't walk the AST at all if there are no functions configured --- lib/credo/check/warning/forbidden_function.ex | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/credo/check/warning/forbidden_function.ex b/lib/credo/check/warning/forbidden_function.ex index 8f245c875..8bd32c338 100644 --- a/lib/credo/check/warning/forbidden_function.ex +++ b/lib/credo/check/warning/forbidden_function.ex @@ -1,5 +1,6 @@ defmodule Credo.Check.Warning.ForbiddenFunction do use Credo.Check, + id: "EX5030", base_priority: :high, category: :warning, param_defaults: [ @@ -35,11 +36,15 @@ defmodule Credo.Check.Warning.ForbiddenFunction do @doc false @impl Credo.Check def run(%SourceFile{} = source_file, params \\ []) do - issue_meta = IssueMeta.for(source_file, params) - functions = Params.get(params, :functions, __MODULE__) - forbidden_map = build_forbidden_map(functions) + 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)) - Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, forbidden_map)) + [] -> + [] + end end defp build_forbidden_map(functions) do From f7c135795bd2a0cf26bc41b58f171c93c6f4f753 Mon Sep 17 00:00:00 2001 From: Tyler Young Date: Sat, 7 Feb 2026 07:41:03 -0600 Subject: [PATCH 3/3] Cope with non-atoms in module parts, like __MODULE__ --- lib/credo/check/warning/forbidden_function.ex | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/credo/check/warning/forbidden_function.ex b/lib/credo/check/warning/forbidden_function.ex index 8bd32c338..a945309c1 100644 --- a/lib/credo/check/warning/forbidden_function.ex +++ b/lib/credo/check/warning/forbidden_function.ex @@ -69,8 +69,15 @@ defmodule Credo.Check.Warning.ForbiddenFunction do forbidden_map ) when is_atom(function) and is_list(module_parts) do - module = Module.concat(module_parts) - {ast, append_issue_if_forbidden({module, function}, forbidden_map, issues, issue_meta, meta)} + 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}