refactor: simplify and sanitize filter_ilike

This commit is contained in:
simon lehericey 2024-11-12 17:40:55 +01:00
parent 343ad1a81c
commit 50677e982c
No known key found for this signature in database
GPG key ID: CDE670D827C7B3C5
2 changed files with 30 additions and 27 deletions

View file

@ -28,10 +28,11 @@ module DossierFilteringConcern
end 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) 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| scope :filter_array_enum, lambda { |table, column, values|
@ -39,5 +40,7 @@ module DossierFilteringConcern
q = Array.new(values.count, "(#{table_column} = ?)").join(' OR ') q = Array.new(values.count, "(#{table_column} = ?)").join(' OR ')
where(q, *(values. map { |value| "[\"#{value}\"]" })) where(q, *(values. map { |value| "[\"#{value}\"]" }))
} }
def sanitize_sql_like(q) = ActiveRecord::Base.sanitize_sql_like(q)
end end
end end

View file

@ -136,29 +136,6 @@
], ],
"note": "escaped by hand" "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_type": "Cross-Site Scripting",
"warning_code": 2, "warning_code": 2,
@ -216,6 +193,29 @@
], ],
"note": "" "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_type": "Cross-Site Scripting",
"warning_code": 2, "warning_code": 2,
@ -272,6 +272,6 @@
"note": "Current is not a model" "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" "brakeman_version": "6.1.2"
} }