You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Link to Rails application code: not to be disclosed
False Positive
Full warning from Brakeman:
Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Country.where("lower(source_slug) = :country_id OR lower(slug_#{I18n.locale.to_s.split("-").first}) = :country_id", :country_id => params[:new_country_id])
File: app/controllers/some_controller.rb
Line: N
Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Location.joins(:region => :country).select("locations.slug_#{I18n.locale.to_s.split("-").first}, locations.source_slug, COALESCE(locations.slug_#{I18n.locale.to_s.split("-").first}, locations.source_slug) as location_slug, countries.slug_#{I18n.locale.to_s.split("-").first} as country_slug, COALESCE(regions.slug_#{I18n.locale.to_s.split("-").first}, regions.source_slug) as region_slug")
File: app/controllers/some_controller2.rb
Line: N
Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: Country.where(:code => codes).order("name_#{I18n.locale.to_s.split("-").first}")
File: app/helpers/some_helper.rb
Line: N
Relevant code:
Country.where("lower(source_slug) = :country_id OR lower(slug_#{I18n.locale.to_s.split("-").first}) = :country_id",country_id: params[:new_country_id]).first
Location.joins(region: :country).select("locations.slug_#{locale}, locations.source_slug, COALESCE(locations.slug_#{locale}, locations.source_slug) as location_slug, countries.slug_#{locale} as country_slug, COALESCE(regions.slug_#{locale}, regions.source_slug) as region_slug").where("locations.region_id IN (?) OR locations.has_cross_border_landing_page = true",@regions.ids)
Brakeman warns about a potential SQL injection vulnerability, although this could not be the case since Rails checks the input passed to I18n.locale = if it matches one of the I18n.available_locales. If you pass some untrusted input, like I18n.locale = "thisisn'tsafe", then you get an error: I18n::InvalidLocale: "thisisn'tsafe" is not a valid locale. So this cannot be an actual SQL injection.
I can't seem to fix or remove this false positive. I tried adding method sanitized_locale which checks the I18n.locale to a whitelist (in the same file), but Brakeman still sees this as a vulnerability. I tried adding sanitized_locale to the --safe-methods option, but this doesn't work either (it appears these methods are only used for the XSS scan). It seems there are only two ways of removing this false positive now:
Wrapping each occurrence in ApplicationRecord.sanitize_sql(...) (so for example: Country.where(code: codes).order("name_#{ApplicationRecord.sanitize_sql(I18n.locale.to_s.split("-").first)}"). Feels a bit cumbersome as well, leading to long lines (of which then Rubocop complains :p). It also doesn't actually fix the SQL vulnerability if there were actually one.
Anyone has any suggestions for a nice and preferably application-wide fix (or ignore method)?
The text was updated successfully, but these errors were encountered:
Note: the correct way to fix this potential SQL injection vulnerability is probably to wrap each column in ApplicationRecord.connection.quote_column_name. So for example: Country.where(code: codes).order(ApplicationRecord.connection.quote_column_name("name_#{I18n.locale.to_s.split("-").first}")). This works and Brakeman views this as secure (although I wrongfully thought it didn't at first). However it still feels a bit cumbersome to do this for every occurrence when there isn't an actual vulnerability.
Background
Brakeman version: 5.0.1
Rails version: 6.0.3.7
Ruby version: 2.7.3
Link to Rails application code: not to be disclosed
False Positive
Full warning from Brakeman:
Relevant code:
Why might this be a false positive?
Brakeman warns about a potential SQL injection vulnerability, although this could not be the case since Rails checks the input passed to
I18n.locale =
if it matches one of the I18n.available_locales. If you pass some untrusted input, likeI18n.locale = "thisisn'tsafe"
, then you get an error:I18n::InvalidLocale: "thisisn'tsafe" is not a valid locale
. So this cannot be an actual SQL injection.I can't seem to fix or remove this false positive. I tried adding method sanitized_locale which checks the I18n.locale to a whitelist (in the same file), but Brakeman still sees this as a vulnerability. I tried adding sanitized_locale to the
--safe-methods
option, but this doesn't work either (it appears these methods are only used for the XSS scan). It seems there are only two ways of removing this false positive now:ApplicationRecord.sanitize_sql(...)
(so for example:Country.where(code: codes).order("name_#{ApplicationRecord.sanitize_sql(I18n.locale.to_s.split("-").first)}"
). Feels a bit cumbersome as well, leading to long lines (of which then Rubocop complains :p). It also doesn't actually fix the SQL vulnerability if there were actually one.Anyone has any suggestions for a nice and preferably application-wide fix (or ignore method)?
The text was updated successfully, but these errors were encountered: