From 50677e982c769a4a19780d5d1be18945601ca296 Mon Sep 17 00:00:00 2001
From: simon lehericey <mail@simon.lehericey.net>
Date: Tue, 12 Nov 2024 17:40:55 +0100
Subject: [PATCH] refactor: simplify and sanitize filter_ilike

---
 .../concerns/dossier_filtering_concern.rb     |  9 ++--
 config/brakeman.ignore                        | 48 +++++++++----------
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/app/models/concerns/dossier_filtering_concern.rb b/app/models/concerns/dossier_filtering_concern.rb
index 1956b27a0..4baab3a0f 100644
--- a/app/models/concerns/dossier_filtering_concern.rb
+++ b/app/models/concerns/dossier_filtering_concern.rb
@@ -28,10 +28,11 @@ module DossierFilteringConcern
       end
     }
 
-    scope :filter_ilike, lambda { |table, column, values|
+    scope :filter_ilike, lambda { |table, column, search_terms|
+      safe_quoted_terms = search_terms.map { "%#{sanitize_sql_like(_1)}%" }
       table_column = DossierFilterService.sanitized_column(table, column)
-      q = Array.new(values.count, "(#{table_column} ILIKE ?)").join(' OR ')
-      where(q, *(values.map { |value| "%#{value}%" }))
+
+      where("#{table_column} LIKE ANY (ARRAY[?])", safe_quoted_terms)
     }
 
     scope :filter_array_enum, lambda { |table, column, values|
@@ -39,5 +40,7 @@ module DossierFilteringConcern
       q = Array.new(values.count, "(#{table_column} = ?)").join(' OR ')
       where(q, *(values. map { |value| "[\"#{value}\"]" }))
     }
+
+    def sanitize_sql_like(q) = ActiveRecord::Base.sanitize_sql_like(q)
   end
 end
diff --git a/config/brakeman.ignore b/config/brakeman.ignore
index 6891674f2..38482ebdd 100644
--- a/config/brakeman.ignore
+++ b/config/brakeman.ignore
@@ -136,29 +136,6 @@
       ],
       "note": "escaped by hand"
     },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "91ff8031e7c639c95fe6c244867349a72078ef456d8b3507deaf2bdb9bf62fe2",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/concerns/dossier_filtering_concern.rb",
-      "line": 34,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "where(\"#{values.count} OR #{\"(#{DossierFilterService.sanitized_column(table, column)} ILIKE ?)\"}\", *values.map do\n \"%#{value}%\"\n end)",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "DossierFilteringConcern",
-        "method": null
-      },
-      "user_input": "values.count",
-      "confidence": "Medium",
-      "cwe_id": [
-        89
-      ],
-      "note": "filtered by rails query params where(something: ?, values)"
-    },
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 2,
@@ -216,6 +193,29 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "afd2a1a41bd87fa62e065671670bd9bd8cc503ca4cbd3cfdb74a38a794146926",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/concerns/dossier_filtering_concern.rb",
+      "line": 35,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "where(\"#{DossierFilterService.sanitized_column(table, column)} LIKE ANY (ARRAY[?])\", search_terms.map do\n \"%#{sanitize_sql_like(_1)}%\"\n end)",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "DossierFilteringConcern",
+        "method": null
+      },
+      "user_input": "DossierFilterService.sanitized_column(table, column)",
+      "confidence": "Medium",
+      "cwe_id": [
+        89
+      ],
+      "note": "The table and column are escaped, which should make this safe"
+    },
     {
       "warning_type": "Cross-Site Scripting",
       "warning_code": 2,
@@ -272,6 +272,6 @@
       "note": "Current is not a model"
     }
   ],
-  "updated": "2024-11-04 09:56:55 +0100",
+  "updated": "2024-11-12 17:33:07 +0100",
   "brakeman_version": "6.1.2"
 }