Merge pull request #9797 from github/nickrolfe/railties_fix

Ruby: fix defining every dataflow node as a command execution sink
This commit is contained in:
Nick Rolfe
2022-07-12 09:30:55 +01:00
committed by GitHub
6 changed files with 29 additions and 7 deletions

View File

@@ -0,0 +1,6 @@
---
category: minorAnalysis
---
* Fixed a bug causing every expression in the database to be considered a system-command execution sink when calls to any of the following methods exist:
* The `spawn`, `fspawn`, `popen4`, `pspawn`, `system`, `_pspawn` methods and the backtick operator from the `POSIX::spawn` gem.
* The `execute_command`, `rake`, `rails_command`, and `git` methods in `Rails::Generation::Actions`.

View File

@@ -62,9 +62,9 @@ module PosixSpawn {
// is shell interpreted unless there is another argument with a string
// constant value.
override predicate isShellInterpreted(DataFlow::Node arg) {
this.argument(arg) and
not exists(DataFlow::Node otherArg |
otherArg != arg and
this.argument(arg) and
this.argument(otherArg) and
otherArg.asExpr().getConstantValue().isString(_)
)

View File

@@ -43,7 +43,7 @@ module Railties {
override DataFlow::Node getAnArgument() { result = this.getArgument([0, 1]) }
override predicate isShellInterpreted(DataFlow::Node arg) { any() }
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getAnArgument() }
}
/**
@@ -57,6 +57,6 @@ module Railties {
override DataFlow::Node getAnArgument() { result = this.getArgument(0) }
override predicate isShellInterpreted(DataFlow::Node arg) { any() }
override predicate isShellInterpreted(DataFlow::Node arg) { arg = this.getAnArgument() }
}
}

View File

@@ -5,11 +5,13 @@ import codeql.ruby.DataFlow
query predicate systemCalls(
PosixSpawn::SystemCall call, DataFlow::Node arg, boolean shellInterpreted
) {
arg = call.getAnArgument() and
if call.isShellInterpreted(arg) then shellInterpreted = true else shellInterpreted = false
call.isShellInterpreted(arg) and shellInterpreted = true
or
not call.isShellInterpreted(arg) and arg = call.getAnArgument() and shellInterpreted = false
}
query predicate childCalls(PosixSpawn::ChildCall call, DataFlow::Node arg, boolean shellInterpreted) {
arg = call.getAnArgument() and
if call.isShellInterpreted(arg) then shellInterpreted = true else shellInterpreted = false
call.isShellInterpreted(arg) and shellInterpreted = true
or
not call.isShellInterpreted(arg) and arg = call.getAnArgument() and shellInterpreted = false
}

View File

@@ -1,5 +1,14 @@
systemCommandExecutions
| Railties.rb:5:5:5:34 | call to execute_command |
| Railties.rb:6:5:6:37 | call to execute_command |
| Railties.rb:8:5:8:16 | call to rake |
| Railties.rb:10:5:10:27 | call to rails_command |
| Railties.rb:12:5:12:17 | call to git |
shellInterpretedArguments
| Railties.rb:5:5:5:34 | call to execute_command | Railties.rb:5:21:5:25 | :rake |
| Railties.rb:5:5:5:34 | call to execute_command | Railties.rb:5:28:5:33 | "test" |
| Railties.rb:6:5:6:37 | call to execute_command | Railties.rb:6:21:6:26 | :rails |
| Railties.rb:6:5:6:37 | call to execute_command | Railties.rb:6:29:6:36 | "server" |
| Railties.rb:8:5:8:16 | call to rake | Railties.rb:8:10:8:15 | "test" |
| Railties.rb:10:5:10:27 | call to rails_command | Railties.rb:10:19:10:26 | "server" |
| Railties.rb:12:5:12:17 | call to git | Railties.rb:12:9:12:16 | "status" |

View File

@@ -1,5 +1,10 @@
private import ruby
private import codeql.ruby.Concepts
private import codeql.ruby.frameworks.Railties
private import codeql.ruby.DataFlow
query predicate systemCommandExecutions(SystemCommandExecution e) { any() }
query predicate shellInterpretedArguments(SystemCommandExecution e, DataFlow::Node arg) {
e.isShellInterpreted(arg)
}