mirror of
https://github.com/github/codeql.git
synced 2026-04-29 10:45:15 +02:00
add a rb/unsafe-shell-command-construction query
This commit is contained in:
erik-krogh
@@ -0,0 +1,108 @@
|
||||
/**
|
||||
* Provides default sources, sinks and sanitizers for reasoning about
|
||||
* shell command constructed from library input vulnerabilities, as
|
||||
* well as extension points for adding your own.
|
||||
*/
|
||||
|
||||
private import codeql.ruby.DataFlow
|
||||
private import codeql.ruby.DataFlow2
|
||||
private import codeql.ruby.ApiGraphs
|
||||
private import codeql.ruby.frameworks.core.Gem::Gem as Gem
|
||||
private import codeql.ruby.AST as Ast
|
||||
private import codeql.ruby.Concepts as Concepts
|
||||
|
||||
module UnsafeShellCommandConstruction {
|
||||
/** A source for shell command constructed from library input vulnerabilities. */
|
||||
abstract class Source extends DataFlow::Node { }
|
||||
|
||||
/** An input parameter to a gem seen as a source. */
|
||||
private class LibraryInputAsSource extends Source instanceof DataFlow::ParameterNode {
|
||||
LibraryInputAsSource() {
|
||||
this = Gem::getALibraryInput() and
|
||||
// we exclude arguments named `cmd` or similar, as they seem to execute commands on purpose
|
||||
not exists(string name | name = super.getName() |
|
||||
name = ["cmd", "command"]
|
||||
or
|
||||
name.regexpMatch(".*(Cmd|Command)$")
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** A sink for shell command constructed from library input vulnerabilities. */
|
||||
abstract class Sink extends DataFlow::Node {
|
||||
/** Gets a description of how the string in this sink was constructed. */
|
||||
abstract string describe();
|
||||
|
||||
/** Gets the dataflow node where the string is constructed. */
|
||||
DataFlow::Node getStringConstruction() { result = this }
|
||||
|
||||
/** Gets the dataflow node that executed the string as a shell command. */
|
||||
abstract DataFlow::Node getCommandExecution();
|
||||
}
|
||||
|
||||
/** A dataflow-configuration for tracking flow from various string constructions to places where those strings are executed as shell commands. */
|
||||
class TrackSystemCommand extends DataFlow2::Configuration {
|
||||
TrackSystemCommand() { this = "StringConcatAsSink::TrackSystemCommand" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
source instanceof TaintedFormat::PrintfStyleCall
|
||||
or
|
||||
source.asExpr().getExpr() =
|
||||
any(Ast::StringLiteral lit |
|
||||
lit.getComponent(_) instanceof Ast::StringInterpolationComponent
|
||||
)
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(Concepts::SystemCommandExecution s | s.isShellInterpreted(sink))
|
||||
}
|
||||
}
|
||||
|
||||
/** Holds if the string constructed at `source` is executed at `shellExec` */
|
||||
predicate isUsedAsShellCommand(DataFlow::Node source, Concepts::SystemCommandExecution shellExec) {
|
||||
any(TrackSystemCommand conf)
|
||||
.hasFlow(source, any(DataFlow::Node arg | shellExec.isShellInterpreted(arg)))
|
||||
}
|
||||
|
||||
/**
|
||||
* A string constructed from a string-literal (e.g. `"foo #{sink}"`),
|
||||
* where the resulting string ends up being executed as a shell command.
|
||||
*/
|
||||
class StringFormatAsSink extends Sink {
|
||||
Concepts::SystemCommandExecution s;
|
||||
Ast::StringLiteral lit;
|
||||
|
||||
StringFormatAsSink() {
|
||||
isUsedAsShellCommand(any(DataFlow::Node n | n.asExpr().getExpr() = lit), s) and
|
||||
this.asExpr().getExpr() = lit.getComponent(_)
|
||||
}
|
||||
|
||||
override string describe() { result = "string construction" }
|
||||
|
||||
override DataFlow::Node getCommandExecution() { result = s }
|
||||
|
||||
override DataFlow::Node getStringConstruction() { result.asExpr().getExpr() = lit }
|
||||
}
|
||||
|
||||
import codeql.ruby.security.TaintedFormatStringSpecific as TaintedFormat
|
||||
|
||||
/**
|
||||
* A string constructed from a printf-style call,
|
||||
* where the resulting string ends up being executed as a shell command.
|
||||
*/
|
||||
class TaintedFormatStringAsSink extends Sink {
|
||||
Concepts::SystemCommandExecution s;
|
||||
TaintedFormat::PrintfStyleCall call;
|
||||
|
||||
TaintedFormatStringAsSink() {
|
||||
isUsedAsShellCommand(call, s) and
|
||||
this = [call.getFormatArgument(_), call.getFormatString()]
|
||||
}
|
||||
|
||||
override string describe() { result = "formatted string" }
|
||||
|
||||
override DataFlow::Node getCommandExecution() { result = s }
|
||||
|
||||
override DataFlow::Node getStringConstruction() { result = call }
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,35 @@
|
||||
/**
|
||||
* Provides a taint tracking configuration for reasoning about shell command
|
||||
* constructed from library input vulnerabilities
|
||||
*
|
||||
* Note, for performance reasons: only import this file if `Configuration` is needed,
|
||||
* otherwise `UnsafeShellCommandConstructionCustomizations` should be imported instead.
|
||||
*/
|
||||
|
||||
import codeql.ruby.DataFlow
|
||||
import UnsafeShellCommandConstructionCustomizations::UnsafeShellCommandConstruction
|
||||
private import codeql.ruby.TaintTracking
|
||||
private import CommandInjectionCustomizations::CommandInjection as CommandInjection
|
||||
private import codeql.ruby.dataflow.BarrierGuards
|
||||
|
||||
/**
|
||||
* A taint-tracking configuration for detecting shell command constructed from library input vulnerabilities.
|
||||
*/
|
||||
class Configuration extends TaintTracking::Configuration {
|
||||
Configuration() { this = "UnsafeShellCommandConstruction" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof Source }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
node instanceof CommandInjection::Sanitizer or // using all sanitizers from `rb/command-injection`
|
||||
node instanceof StringConstCompareBarrier or
|
||||
node instanceof StringConstArrayInclusionCallBarrier
|
||||
}
|
||||
|
||||
// override to require the path doesn't have unmatched return steps
|
||||
override DataFlow::FlowFeature getAFeature() {
|
||||
result instanceof DataFlow::FeatureHasSourceCallContext
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,26 @@
|
||||
/**
|
||||
* @name Unsafe shell command constructed from library input
|
||||
* @description Using externally controlled strings in a command line may allow a malicious
|
||||
* user to change the meaning of the command.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @security-severity 6.3
|
||||
* @precision high
|
||||
* @id rb/shell-command-constructed-from-input
|
||||
* @tags correctness
|
||||
* security
|
||||
* external/cwe/cwe-078
|
||||
* external/cwe/cwe-088
|
||||
* external/cwe/cwe-073
|
||||
*/
|
||||
|
||||
import codeql.ruby.security.UnsafeShellCommandConstructionQuery
|
||||
import DataFlow::PathGraph
|
||||
|
||||
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
|
||||
where
|
||||
config.hasFlowPath(source, sink) and
|
||||
sinkNode = sink.getNode()
|
||||
select sinkNode.getStringConstruction(), source, sink,
|
||||
"This " + sinkNode.describe() + " which depends on $@ is later used in a $@.", source.getNode(),
|
||||
"library input", sinkNode.getCommandExecution(), "shell command"
|
||||
@@ -0,0 +1,40 @@
|
||||
edges
|
||||
| impl/sub/notImported.rb:2:12:2:17 | target : | impl/sub/notImported.rb:3:19:3:27 | #{...} |
|
||||
| impl/sub/other2.rb:2:12:2:17 | target : | impl/sub/other2.rb:3:19:3:27 | #{...} |
|
||||
| impl/sub/other.rb:2:12:2:17 | target : | impl/sub/other.rb:3:19:3:27 | #{...} |
|
||||
| impl/unsafeShell.rb:2:12:2:17 | target : | impl/unsafeShell.rb:3:19:3:27 | #{...} |
|
||||
| impl/unsafeShell.rb:6:12:6:12 | x : | impl/unsafeShell.rb:7:32:7:32 | x |
|
||||
| impl/unsafeShell.rb:15:47:15:64 | innocent_file_path : | impl/unsafeShell.rb:20:21:20:41 | #{...} |
|
||||
| impl/unsafeShell.rb:23:15:23:23 | file_path : | impl/unsafeShell.rb:26:19:26:30 | #{...} |
|
||||
| impl/unsafeShell.rb:33:12:33:17 | target : | impl/unsafeShell.rb:34:19:34:27 | #{...} |
|
||||
| impl/unsafeShell.rb:37:10:37:10 | x : | impl/unsafeShell.rb:38:19:38:22 | #{...} |
|
||||
nodes
|
||||
| impl/sub/notImported.rb:2:12:2:17 | target : | semmle.label | target : |
|
||||
| impl/sub/notImported.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
|
||||
| impl/sub/other2.rb:2:12:2:17 | target : | semmle.label | target : |
|
||||
| impl/sub/other2.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
|
||||
| impl/sub/other.rb:2:12:2:17 | target : | semmle.label | target : |
|
||||
| impl/sub/other.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
|
||||
| impl/unsafeShell.rb:2:12:2:17 | target : | semmle.label | target : |
|
||||
| impl/unsafeShell.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
|
||||
| impl/unsafeShell.rb:6:12:6:12 | x : | semmle.label | x : |
|
||||
| impl/unsafeShell.rb:7:32:7:32 | x | semmle.label | x |
|
||||
| impl/unsafeShell.rb:15:47:15:64 | innocent_file_path : | semmle.label | innocent_file_path : |
|
||||
| impl/unsafeShell.rb:20:21:20:41 | #{...} | semmle.label | #{...} |
|
||||
| impl/unsafeShell.rb:23:15:23:23 | file_path : | semmle.label | file_path : |
|
||||
| impl/unsafeShell.rb:26:19:26:30 | #{...} | semmle.label | #{...} |
|
||||
| impl/unsafeShell.rb:33:12:33:17 | target : | semmle.label | target : |
|
||||
| impl/unsafeShell.rb:34:19:34:27 | #{...} | semmle.label | #{...} |
|
||||
| impl/unsafeShell.rb:37:10:37:10 | x : | semmle.label | x : |
|
||||
| impl/unsafeShell.rb:38:19:38:22 | #{...} | semmle.label | #{...} |
|
||||
subpaths
|
||||
#select
|
||||
| impl/sub/notImported.rb:3:14:3:28 | "cat #{...}" | impl/sub/notImported.rb:2:12:2:17 | target : | impl/sub/notImported.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/sub/notImported.rb:2:12:2:17 | target | library input | impl/sub/notImported.rb:3:5:3:34 | call to popen | shell command |
|
||||
| impl/sub/other2.rb:3:14:3:28 | "cat #{...}" | impl/sub/other2.rb:2:12:2:17 | target : | impl/sub/other2.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/sub/other2.rb:2:12:2:17 | target | library input | impl/sub/other2.rb:3:5:3:34 | call to popen | shell command |
|
||||
| impl/sub/other.rb:3:14:3:28 | "cat #{...}" | impl/sub/other.rb:2:12:2:17 | target : | impl/sub/other.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/sub/other.rb:2:12:2:17 | target | library input | impl/sub/other.rb:3:5:3:34 | call to popen | shell command |
|
||||
| impl/unsafeShell.rb:3:14:3:28 | "cat #{...}" | impl/unsafeShell.rb:2:12:2:17 | target : | impl/unsafeShell.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:2:12:2:17 | target | library input | impl/unsafeShell.rb:3:5:3:34 | call to popen | shell command |
|
||||
| impl/unsafeShell.rb:7:14:7:33 | call to sprintf | impl/unsafeShell.rb:6:12:6:12 | x : | impl/unsafeShell.rb:7:32:7:32 | x | This formatted string which depends on $@ is later used in a $@. | impl/unsafeShell.rb:6:12:6:12 | x | library input | impl/unsafeShell.rb:8:5:8:25 | call to popen | shell command |
|
||||
| impl/unsafeShell.rb:20:14:20:42 | "which #{...}" | impl/unsafeShell.rb:15:47:15:64 | innocent_file_path : | impl/unsafeShell.rb:20:21:20:41 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:15:47:15:64 | innocent_file_path | library input | impl/unsafeShell.rb:20:5:20:48 | call to popen | shell command |
|
||||
| impl/unsafeShell.rb:26:14:26:31 | "cat #{...}" | impl/unsafeShell.rb:23:15:23:23 | file_path : | impl/unsafeShell.rb:26:19:26:30 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:23:15:23:23 | file_path | library input | impl/unsafeShell.rb:26:5:26:37 | call to popen | shell command |
|
||||
| impl/unsafeShell.rb:34:14:34:28 | "cat #{...}" | impl/unsafeShell.rb:33:12:33:17 | target : | impl/unsafeShell.rb:34:19:34:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:33:12:33:17 | target | library input | impl/unsafeShell.rb:34:5:34:34 | call to popen | shell command |
|
||||
| impl/unsafeShell.rb:38:14:38:23 | "cat #{...}" | impl/unsafeShell.rb:37:10:37:10 | x : | impl/unsafeShell.rb:38:19:38:22 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:37:10:37:10 | x | library input | impl/unsafeShell.rb:38:5:38:29 | call to popen | shell command |
|
||||
@@ -0,0 +1 @@
|
||||
queries/security/cwe-078/UnsafeShellCommandConstruction.ql
|
||||
@@ -0,0 +1,6 @@
|
||||
class Foobar
|
||||
def foo1(target)
|
||||
IO.popen("cat #{target}", "w") # NOT OK - everything assumed to be imported...
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
class Foobar
|
||||
def foo1(target)
|
||||
IO.popen("cat #{target}", "w") # NOT OK
|
||||
end
|
||||
end
|
||||
|
||||
require 'sub/other2'
|
||||
@@ -0,0 +1,5 @@
|
||||
class Foobar
|
||||
def foo1(target)
|
||||
IO.popen("cat #{target}", "w") # NOT OK
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,45 @@
|
||||
class Foobar
|
||||
def foo1(target)
|
||||
IO.popen("cat #{target}", "w") # NOT OK
|
||||
end
|
||||
|
||||
def foo2(x)
|
||||
format = sprintf("cat %s", x) # NOT OK
|
||||
IO.popen(format, "w")
|
||||
end
|
||||
|
||||
def fileRead1(path)
|
||||
File.read(path) # OK
|
||||
end
|
||||
|
||||
def my_exec(cmd, command, myCmd, myCommand, innocent_file_path)
|
||||
IO.popen("which #{cmd}", "w") # OK - the parameter is named `cmd`, so it's meant to be a command
|
||||
IO.popen("which #{command}", "w") # OK - the parameter is named `command`, so it's meant to be a command
|
||||
IO.popen("which #{myCmd}", "w") # OK - the parameter is named `myCmd`, so it's meant to be a command
|
||||
IO.popen("which #{myCommand}", "w") # OK - the parameter is named `myCommand`, so it's meant to be a command
|
||||
IO.popen("which #{innocent_file_path}", "w") # NOT OK - the parameter is named `innocent_file_path`, so it's not meant to be a command
|
||||
end
|
||||
|
||||
def escaped(file_path)
|
||||
IO.popen("cat #{file_path.shellescape}", "w") # OK - the parameter is escaped
|
||||
|
||||
IO.popen("cat #{file_path}", "w") # NOT OK - the parameter is not escaped
|
||||
end
|
||||
end
|
||||
|
||||
require File.join(File.dirname(__FILE__), 'sub', 'other')
|
||||
|
||||
class Foobar2
|
||||
def foo1(target)
|
||||
IO.popen("cat #{target}", "w") # NOT OK
|
||||
end
|
||||
|
||||
def id(x)
|
||||
IO.popen("cat #{x}", "w") # NOT OK - the parameter is not a constant.
|
||||
return x
|
||||
end
|
||||
|
||||
def thisIsSafe()
|
||||
IO.popen("echo #{id('foo')}", "w") # OK - only using constants.
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,5 @@
|
||||
Gem::Specification.new do |s|
|
||||
s.name = 'unsafe-shell'
|
||||
s.require_path = "impl"
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user