Catch SQL.query! (#155)

* Create failing unit tests to cover the SQL.query! function #153
* Update Sobelow.SQL.Query to test for both query() and query!(), similar to Sobelow.Traversal.FileModule #153
* update the Query moduledoc

Author: iarekk

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

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