Project

General

Profile

Refactor #23300

Do not use string interpolation when composing SQL queries.

Added by Martin Povolny over 4 years ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Rails
Target version:
-
Difficulty:
Triaged:
No
Bugzilla link:
Pull request:
https://github.com/theforeman/foreman/pull/8979
Fixed in Releases:
3.2.0
Found in Releases:
Red Hat JIRA:

Description

Using string interpolation when composing SQL queries is just one step away from creating a security issue. It's against the Rails best practices to do so. Doing so actually results into Brakeman complaining loudly.

Task: replace string interpolation with use of parameterization of queries and/or AREL.

Related issues

Related to Foreman - Tracker #21834: Rails 5.2 upgrade tasksClosed

Related to Foreman - Refactor #23234: remove friendly_id <5.0 workaroundsClosed
Related to Foreman - Tracker #24837: Rails 6.0 TrackerClosed

Related to Foreman - Refactor #29520: Wrap sql in Arel.sql() where neededClosed
Blocks Foreman - Tracker #28570: Rails 6.1 TrackerNew

Associated revisions

Revision 09c865a3 (diff)
Added by Leos Stejskal 11 months ago

Fixes #23300 - Brakeman SQL injections

Fix all foundings of SQL injections by
Brakeman: "brakeman --test SQL"

History

#1 Updated by Marek Hulán over 4 years ago

  • Status changed from New to Need more information

Could you share the list of such places? Or is that based on brakeman scan only? Was it just Foreman core or also some plugins that you've scanned?

#2 Updated by Anonymous over 4 years ago

Brakeman is there: http://ci.theforeman.org/job/test_brakeman (although that's going to be deleted soon). The Rails 5.2 warnings can be seen in the new deprecations in https://github.com/theforeman/foreman/pull/5428

#3 Updated by Martin Povolny over 4 years ago

I started with Brakeman scan and `grep` and with Foreman only and did not spend much time on this yet.

I think that basic checking should be done on regular basis possibly as part of the CI and also for plugins. Brakeman can be used and/or services such as Hakiri (https://hakiri.io/).

I don't have a list of issues. Initial one can be obtained by running Brakeman.

In my opinion as a starting point all issues reported by Brakeman should be fixed or marked as false positives in the Brakeman config file (to be included with Foreman).

#4 Updated by Anonymous over 4 years ago

#5 Updated by Anonymous over 4 years ago

  • Tracker changed from Bug to Refactor

#6 Updated by Anonymous over 4 years ago

#7 Updated by Anonymous over 4 years ago

  • Pull request https://github.com/theforeman/foreman/pull/5367 added

#8 Updated by Anonymous over 4 years ago

#9 Updated by Anonymous over 3 years ago

  • Status changed from Need more information to New
  • Pull request deleted (https://github.com/theforeman/foreman/pull/5367)

#10 Updated by Anonymous over 3 years ago

  • Related to Bug #26414: Api error when querying LDAP users added

#11 Updated by Anonymous almost 3 years ago

  • Related to deleted (Bug #26414: Api error when querying LDAP users )

#12 Updated by Anonymous almost 3 years ago

#13 Updated by Anonymous almost 3 years ago

#14 Updated by Anonymous almost 3 years ago

#15 Updated by Anonymous over 2 years ago

#16 Updated by The Foreman Bot about 1 year ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/8979 added

#17 Updated by The Foreman Bot 11 months ago

  • Fixed in Releases 3.2.0 added

#18 Updated by Leos Stejskal 11 months ago

  • Status changed from Ready For Testing to Closed

#19 Updated by Amit Upadhye 10 months ago

  • Category set to Rails

Also available in: Atom PDF