Skip to content

Commit

Permalink
update to rails unsafe reflection methods (semgrep#3265)
Browse files Browse the repository at this point in the history
* update to rails unsafe reflection methods

* change from deep ellipsis to focus metavariable

---------

Co-authored-by: Kurt Boberg <98792107+kurt-r2c@users.noreply.github.com>
  • Loading branch information
normprovost and kurt-r2c authored Jan 9, 2024
1 parent 353cd3a commit 138966d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ def dynamic_method_invocations
# ruleid: check-unsafe-reflection-methods
Kernel.tap(&params[:method].to_sym)
User.method("#{User.first.some_method_thing}_stuff")
user_input_value = params[:my_user_input]
# ruleid: check-unsafe-reflection-methods
anything.tap(&user_input_value.to_sym)
# ruleid: check-unsafe-reflection-methods
anything_else.tap { |thing| thing + user_input_value() }
end

def dynamic_method_invocations_ok
Expand All @@ -17,6 +22,9 @@ def dynamic_method_invocations_ok
SomeClass.method("some_method").("some_argument")
# ok: check-unsafe-reflection-methods
Kernel.tap("SomeClass".to_sym)
user_input_value = params[:my_user_input]
# ok: check-unsafe-reflection-methods
user_input_value.tap("some_method")
end

end
16 changes: 12 additions & 4 deletions ruby/rails/security/brakeman/check-unsafe-reflection-methods.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,18 @@ rules:
- pattern-either:
- pattern-inside: |
$X. ... .to_proc
- pattern-inside: |
$Y.method(...)
- pattern-inside: |
$Y.tap(...)
- patterns:
- pattern-inside: |
$Y.method($Z)
- focus-metavariable: $Z
- patterns:
- pattern-inside: |
$Y.tap($Z)
- focus-metavariable: $Z
- patterns:
- pattern-inside: |
$Y.tap{ |$ANY| $Z }
- focus-metavariable: $Z
message: Found user-controllable input to a reflection method. This may allow a user to alter program
behavior and potentially execute arbitrary instructions in the context of the process. Do not provide
arbitrary user input to `tap`, `method`, or `to_proc`
Expand Down

0 comments on commit 138966d

Please sign in to comment.