search

presidentbeef / brakeman

A static analysis security vulnerability scanner for Ruby on Rails applications
https://brakemanscanner.org/
Other
7.02k stars 732 forks source link

False positive when methods chained on permitted params #1885

Open bschmeck opened 3 days ago

bschmeck commented 3 days ago

Background

Brakeman version: 6.2.2 Rails version: 8.0.0 Ruby version: 3.3.5

False Positive

Full warning from Brakeman:

Confidence: High
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: MyModel.where(params.permit(:foo, :bar).slice(:foo, :bar).compact_blank)

Relevant code:

MyModel.where(params.permit(:foo, :bar).slice(:foo, :bar)) # no warning
MyModel.where(params.permit(:foo, :bar).slice(:foo, :bar).compact_blank) # warning

The compact_blank method returns a new ActionController::Parameters object, but that new object retains the permitted status from our earlier call to permit. There is a list of permitted method calls defined here but compact_blank is not listed there. It probably shouldn't be, because compact_blank on its own does not make the call safe, but being chained after permit ought to.

Is there a way to mark methods that are chained after permit as safe?