diff --git a/lib/sobelow/sql/query.ex b/lib/sobelow/sql/query.ex index bbfcb7c..3cc575a 100644 --- a/lib/sobelow/sql/query.ex +++ b/lib/sobelow/sql/query.ex @@ -3,7 +3,8 @@ defmodule Sobelow.SQL.Query do # SQL Injection in Query This submodule of the `SQL` module checks for SQL injection - vulnerabilities through usage of the `Ecto.Adapters.SQL.query`. + vulnerabilities through usage of the `Ecto.Adapters.SQL.query` + and `Ecto.Adapters.SQL.query!`. Ensure that the query is parameterized and not user-controlled. @@ -13,27 +14,32 @@ defmodule Sobelow.SQL.Query do """ @uid 17 @finding_type "SQL.Query: SQL injection" + @query_funcs [:query, :query!] use Sobelow.Finding def run(fun, meta_file) do confidence = if !meta_file.is_controller?, do: :low - Finding.init(@finding_type, meta_file.filename, confidence) - |> Finding.multi_from_def(fun, parse_sql_def(fun)) - |> Enum.each(&Print.add_finding(&1)) - - Finding.init(@finding_type, meta_file.filename, confidence) - |> Finding.multi_from_def(fun, parse_repo_query_def(fun)) - |> Enum.each(&Print.add_finding(&1)) + Enum.each(@query_funcs, fn query_func -> + Finding.init(@finding_type, meta_file.filename, confidence) + |> Finding.multi_from_def(fun, parse_sql_def(fun, query_func)) + |> Enum.each(&Print.add_finding(&1)) + end) + + Enum.each(@query_funcs, fn query_func -> + Finding.init(@finding_type, meta_file.filename, confidence) + |> Finding.multi_from_def(fun, parse_repo_query_def(fun, query_func)) + |> Enum.each(&Print.add_finding(&1)) + end) end ## query(repo, sql, params \\ [], opts \\ []) - def parse_sql_def(fun) do - Parse.get_fun_vars_and_meta(fun, 1, :query, :SQL) + def parse_sql_def(fun, type) do + Parse.get_fun_vars_and_meta(fun, 1, type, :SQL) end - def parse_repo_query_def(fun) do - Parse.get_fun_vars_and_meta(fun, 0, :query, :Repo) + def parse_repo_query_def(fun, type) do + Parse.get_fun_vars_and_meta(fun, 0, type, :Repo) end end diff --git a/test/sql/query_test.exs b/test/sql/query_test.exs index 794ef4f..864ccc6 100644 --- a/test/sql/query_test.exs +++ b/test/sql/query_test.exs @@ -3,51 +3,61 @@ defmodule SobelowTest.SQL.QueryTest do import Sobelow, only: [is_vuln?: 1] alias Sobelow.SQL.Query + @query_funcs [:query, :query!] + test "SQL injection in `SQL`" do - func = """ - def query(%{"sql" => sql}) do - SQL.query(Repo, sql, []) - end - """ + Enum.each(@query_funcs, fn query_func -> + func = """ + def query(%{"sql" => sql}) do + SQL.#{query_func}(Repo, sql, []) + end + """ - {_, ast} = Code.string_to_quoted(func) + {_, ast} = Code.string_to_quoted(func) - assert Query.parse_sql_def(ast) |> is_vuln? + assert Query.parse_sql_def(ast, query_func) |> is_vuln? + end) end test "Safe `SQL`" do - func = """ - def query(%{"sql" => sql}) do - SQL.query(Repo, "SELECT * FROM users", []) - end - """ + Enum.each(@query_funcs, fn query_func -> + func = """ + def query(%{"sql" => sql}) do + SQL.#{query_func}(Repo, "SELECT * FROM users", []) + end + """ - {_, ast} = Code.string_to_quoted(func) + {_, ast} = Code.string_to_quoted(func) - refute Query.parse_sql_def(ast) |> is_vuln? + refute Query.parse_sql_def(ast, query_func) |> is_vuln? + end) end test "SQL injection in `Repo`" do - func = """ - def query(%{"sql" => sql}) do - Repo.query(sql) - end - """ + Enum.each(@query_funcs, fn query_func -> + func = """ + def query(%{"sql" => sql}) do + Repo.#{query_func}(sql) + end + """ - {_, ast} = Code.string_to_quoted(func) + {_, ast} = Code.string_to_quoted(func) - assert Query.parse_repo_query_def(ast) |> is_vuln? + assert Query.parse_repo_query_def(ast, query_func) |> is_vuln? + end) end test "safe `Repo`" do - func = """ - def query(%{"sql" => sql}) do - Repo.query("SELECT * FROM users") - end - """ + Enum.each(@query_funcs, fn query_func -> + func = """ + def query(%{"sql" => sql}) do + Repo.#{query_func}("SELECT * FROM users") + end + """ - {_, ast} = Code.string_to_quoted(func) + {_, ast} = Code.string_to_quoted(func) - refute Query.parse_repo_query_def(ast) |> is_vuln? + refute Query.parse_repo_query_def(ast, query_func) |> is_vuln? + end) end end