Skip to content

Commit f6215d4

Browse files
hmachmac
committed
Ruby: Add rb/tainted-format-string query
1 parent 4bf35ad commit f6215d4

File tree

9 files changed

+270
-0
lines changed

9 files changed

+270
-0
lines changed

‎ruby/ql/lib/codeql/ruby/security/TaintedFormatStringCustomizations.qll

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* format injections, as well as extension points for adding your own.
4+
*/
5+
6+
private import ruby
7+
private import codeql.ruby.DataFlow
8+
private import codeql.ruby.dataflow.RemoteFlowSources
9+
private import codeql.ruby.ApiGraphs
10+
11+
module TaintedFormatString {
12+
/**
13+
* A data flow source for format injections.
14+
*/
15+
abstract class Source extends DataFlow::Node { }
16+
17+
/**
18+
* A data flow sink for format injections.
19+
*/
20+
abstract class Sink extends DataFlow::Node { }
21+
22+
/**
23+
* A sanitizer for format injections.
24+
*/
25+
abstract class Sanitizer extends DataFlow::Node { }
26+
27+
/** A source of remote user input, considered as a flow source for format injection. */
28+
class RemoteSource extends Source {
29+
RemoteSource() { this instanceof RemoteFlowSource }
30+
}
31+
32+
/**
33+
* A format argument to a printf-like function, considered as a flow sink for format injection.
34+
*/
35+
class FormatSink extends Sink {
36+
FormatSink() {
37+
exists(PrintfCall printf |
38+
this = printf.getFormatString() and
39+
// exclude trivial case where there are no arguments to interpolate
40+
exists(printf.getFormatArgument(_))
41+
)
42+
}
43+
}
44+
45+
/**
46+
* A call to `printf` or `sprintf`.
47+
*/
48+
abstract class PrintfCall extends DataFlow::CallNode {
49+
// We assume that most printf-like calls have the signature f(format_string, args...)
50+
DataFlow::Node getFormatString() { result = this.getArgument(0) }
51+
52+
DataFlow::Node getFormatArgument(int n) { n > 0 and result = this.getArgument(n) }
53+
}
54+
55+
class KernelPrintfCall extends PrintfCall {
56+
KernelPrintfCall() {
57+
this = API::getTopLevelMember("Kernel").getAMethodCall("printf")
58+
or
59+
this.asExpr().getExpr() instanceof UnknownMethodCall and
60+
this.getMethodName() = "printf"
61+
}
62+
63+
// Kernel#printf supports two signatures:
64+
// printf(io, string, ...)
65+
// printf(string, ...)
66+
override DataFlow::Node getFormatString() { result = this.getArgument([0, 1]) }
67+
}
68+
69+
class KernelSprintfCall extends PrintfCall {
70+
KernelSprintfCall() {
71+
this = API::getTopLevelMember("Kernel").getAMethodCall("sprintf")
72+
or
73+
this.asExpr().getExpr() instanceof UnknownMethodCall and
74+
this.getMethodName() = "sprintf"
75+
}
76+
}
77+
78+
class IOPrintfCall extends PrintfCall {
79+
IOPrintfCall() { this = API::getTopLevelMember("IO").getInstance().getAMethodCall("printf") }
80+
}
81+
}

‎ruby/ql/lib/codeql/ruby/security/TaintedFormatStringQuery.qll

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about format
3+
* injections.
4+
*
5+
*
6+
* Note, for performance reasons: only import this file if
7+
* `TaintedFormatString::Configuration` is needed, otherwise
8+
* `TaintedFormatStringCustomizations` should be imported instead.
9+
*/
10+
11+
import ruby
12+
import codeql.ruby.DataFlow
13+
import codeql.ruby.TaintTracking
14+
import TaintedFormatStringCustomizations::TaintedFormatString
15+
16+
/**
17+
* A taint-tracking configuration for format injections.
18+
*/
19+
class Configuration extends TaintTracking::Configuration {
20+
Configuration() { this = "TaintedFormatString" }
21+
22+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
23+
24+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
25+
26+
override predicate isSanitizer(DataFlow::Node node) {
27+
super.isSanitizer(node) or
28+
node instanceof Sanitizer
29+
}
30+
}

‎ruby/ql/src/queries/security/cwe-134/TaintedFormatString.qhelp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Methods like <code>Kernel.printf</code> accept a format string that is used to format
9+
the remaining arguments by providing inline format specifiers. If the format string
10+
contains unsanitized input from an untrusted source, then that string may contain
11+
unexpected format specifiers that cause garbled output or throw an exception.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
Either sanitize the input before including it in the format string, or use a
18+
<code>%s</code> specifier in the format string, and pass the untrusted data as corresponding
19+
argument.
20+
</p>
21+
</recommendation>
22+
23+
<example>
24+
<p>
25+
The following program snippet logs information about an unauthorized access attempt. The
26+
log message includes the user name, and the user's IP address is passed as an additional
27+
argument to <code>Kernel.printf</code> to be appended to the message:
28+
</p>
29+
<sample src="examples/tainted_format_string_bad.rb"/>
30+
<p>
31+
However, if a malicious user provides a format specified such as <code>%s</code> as their
32+
user name, <code>Kernel.printf</code> throw an exception that there are too few arguments
33+
to satisfy the format. This can result in denial of service or leaking of internal
34+
information to the attacker via a stack trace.
35+
</p>
36+
<p>
37+
Instead, the user name should be included using the <code>%s</code> specifier:
38+
</p>
39+
<sample src="examples/tainted_format_string_good.rb"/>
40+
41+
<p>
42+
Alternatively, a method such as <code>Kernel.puts</code> should be used, which does not
43+
apply string formatting to its arguments.
44+
</p>
45+
</example>
46+
47+
<references>
48+
<li>Ruby documentation for <a href="https://docs.ruby-lang.org/en/3.1/Kernel.html#method-i-sprintf">format strings</a>.</li>
49+
<li>Common Weakness Enumeration: <a href="https://cwe.mitre.org/data/definitions/134.html">CWE-134</a>.</li>
50+
</references>
51+
</qhelp>

‎ruby/ql/src/queries/security/cwe-134/TaintedFormatString.ql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Use of externally-controlled format string
3+
* @description Using external input in format strings can lead to garbled output.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @security-severity 7.3
7+
* @precision high
8+
* @id rb/tainted-format-string
9+
* @tags security
10+
* external/cwe/cwe-134
11+
*/
12+
13+
import ruby
14+
import codeql.ruby.security.TaintedFormatStringQuery
15+
import DataFlow::PathGraph
16+
17+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
18+
where cfg.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "$@ flows here and is used in a format string.",
20+
source.getNode(), "User-provided value"

‎ruby/ql/src/queries/security/cwe-134/examples/tainted_format_string_bad.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class UsersController < ActionController::Base
2+
def index
3+
printf("Unauthorised access attempt by #{params[:user]}: %s", request.ip)
4+
end
5+
end

‎ruby/ql/src/queries/security/cwe-134/examples/tainted_format_string_good.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class UsersController < ActionController::Base
2+
def index
3+
printf("Unauthorised access attempt by %s: %s", params[:user], request.ip)
4+
end
5+
end

‎ruby/ql/test/query-tests/security/cwe-134/TaintedFormatString.expected

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
edges
2+
| tainted_format_string.rb:4:12:4:17 | call to params : | tainted_format_string.rb:4:12:4:26 | ...[...] |
3+
| tainted_format_string.rb:5:19:5:24 | call to params : | tainted_format_string.rb:5:19:5:33 | ...[...] |
4+
| tainted_format_string.rb:10:23:10:28 | call to params : | tainted_format_string.rb:10:23:10:37 | ...[...] |
5+
| tainted_format_string.rb:11:30:11:35 | call to params : | tainted_format_string.rb:11:30:11:44 | ...[...] |
6+
| tainted_format_string.rb:13:23:13:28 | call to params : | tainted_format_string.rb:13:23:13:37 | ...[...] |
7+
| tainted_format_string.rb:14:30:14:35 | call to params : | tainted_format_string.rb:14:30:14:44 | ...[...] |
8+
| tainted_format_string.rb:16:27:16:32 | call to params : | tainted_format_string.rb:16:27:16:41 | ...[...] |
9+
| tainted_format_string.rb:17:20:17:25 | call to params : | tainted_format_string.rb:17:20:17:34 | ...[...] |
10+
| tainted_format_string.rb:23:19:23:24 | call to params : | tainted_format_string.rb:23:19:23:33 | ...[...] |
11+
| tainted_format_string.rb:29:32:29:37 | call to params : | tainted_format_string.rb:29:32:29:46 | ...[...] : |
12+
| tainted_format_string.rb:29:32:29:46 | ...[...] : | tainted_format_string.rb:29:12:29:46 | ... + ... |
13+
nodes
14+
| tainted_format_string.rb:4:12:4:17 | call to params : | semmle.label | call to params : |
15+
| tainted_format_string.rb:4:12:4:26 | ...[...] | semmle.label | ...[...] |
16+
| tainted_format_string.rb:5:19:5:24 | call to params : | semmle.label | call to params : |
17+
| tainted_format_string.rb:5:19:5:33 | ...[...] | semmle.label | ...[...] |
18+
| tainted_format_string.rb:10:23:10:28 | call to params : | semmle.label | call to params : |
19+
| tainted_format_string.rb:10:23:10:37 | ...[...] | semmle.label | ...[...] |
20+
| tainted_format_string.rb:11:30:11:35 | call to params : | semmle.label | call to params : |
21+
| tainted_format_string.rb:11:30:11:44 | ...[...] | semmle.label | ...[...] |
22+
| tainted_format_string.rb:13:23:13:28 | call to params : | semmle.label | call to params : |
23+
| tainted_format_string.rb:13:23:13:37 | ...[...] | semmle.label | ...[...] |
24+
| tainted_format_string.rb:14:30:14:35 | call to params : | semmle.label | call to params : |
25+
| tainted_format_string.rb:14:30:14:44 | ...[...] | semmle.label | ...[...] |
26+
| tainted_format_string.rb:16:27:16:32 | call to params : | semmle.label | call to params : |
27+
| tainted_format_string.rb:16:27:16:41 | ...[...] | semmle.label | ...[...] |
28+
| tainted_format_string.rb:17:20:17:25 | call to params : | semmle.label | call to params : |
29+
| tainted_format_string.rb:17:20:17:34 | ...[...] | semmle.label | ...[...] |
30+
| tainted_format_string.rb:23:19:23:24 | call to params : | semmle.label | call to params : |
31+
| tainted_format_string.rb:23:19:23:33 | ...[...] | semmle.label | ...[...] |
32+
| tainted_format_string.rb:29:12:29:46 | ... + ... | semmle.label | ... + ... |
33+
| tainted_format_string.rb:29:32:29:37 | call to params : | semmle.label | call to params : |
34+
| tainted_format_string.rb:29:32:29:46 | ...[...] : | semmle.label | ...[...] : |
35+
subpaths
36+
#select
37+
| tainted_format_string.rb:4:12:4:26 | ...[...] | tainted_format_string.rb:4:12:4:17 | call to params : | tainted_format_string.rb:4:12:4:26 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:4:12:4:17 | call to params | User-provided value |
38+
| tainted_format_string.rb:5:19:5:33 | ...[...] | tainted_format_string.rb:5:19:5:24 | call to params : | tainted_format_string.rb:5:19:5:33 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:5:19:5:24 | call to params | User-provided value |
39+
| tainted_format_string.rb:10:23:10:37 | ...[...] | tainted_format_string.rb:10:23:10:28 | call to params : | tainted_format_string.rb:10:23:10:37 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:10:23:10:28 | call to params | User-provided value |
40+
| tainted_format_string.rb:11:30:11:44 | ...[...] | tainted_format_string.rb:11:30:11:35 | call to params : | tainted_format_string.rb:11:30:11:44 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:11:30:11:35 | call to params | User-provided value |
41+
| tainted_format_string.rb:13:23:13:37 | ...[...] | tainted_format_string.rb:13:23:13:28 | call to params : | tainted_format_string.rb:13:23:13:37 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:13:23:13:28 | call to params | User-provided value |
42+
| tainted_format_string.rb:14:30:14:44 | ...[...] | tainted_format_string.rb:14:30:14:35 | call to params : | tainted_format_string.rb:14:30:14:44 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:14:30:14:35 | call to params | User-provided value |
43+
| tainted_format_string.rb:16:27:16:41 | ...[...] | tainted_format_string.rb:16:27:16:32 | call to params : | tainted_format_string.rb:16:27:16:41 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:16:27:16:32 | call to params | User-provided value |
44+
| tainted_format_string.rb:17:20:17:34 | ...[...] | tainted_format_string.rb:17:20:17:25 | call to params : | tainted_format_string.rb:17:20:17:34 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:17:20:17:25 | call to params | User-provided value |
45+
| tainted_format_string.rb:23:19:23:33 | ...[...] | tainted_format_string.rb:23:19:23:24 | call to params : | tainted_format_string.rb:23:19:23:33 | ...[...] | $@ flows here and is used in a format string. | tainted_format_string.rb:23:19:23:24 | call to params | User-provided value |
46+
| tainted_format_string.rb:29:12:29:46 | ... + ... | tainted_format_string.rb:29:32:29:37 | call to params : | tainted_format_string.rb:29:12:29:46 | ... + ... | $@ flows here and is used in a format string. | tainted_format_string.rb:29:32:29:37 | call to params | User-provided value |

‎ruby/ql/test/query-tests/security/cwe-134/TaintedFormatString.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/security/cwe-134/TaintedFormatString.ql

‎ruby/ql/test/query-tests/security/cwe-134/tainted_format_string.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
class UsersController < ActionController::Base
2+
3+
def show
4+
printf(params[:format], arg) # BAD
5+
Kernel.printf(params[:format], arg) # BAD
6+
7+
printf(params[:format]) # GOOD
8+
Kernel.printf(params[:format]) # GOOD
9+
10+
printf(IO.new(1), params[:format], arg) # BAD
11+
Kernel.printf(IO.new(1), params[:format], arg) # BAD
12+
13+
printf(IO.new(1), params[:format]) # GOOD [FALSE POSITIVE]
14+
Kernel.printf(IO.new(1), params[:format]) # GOOD [FALSE POSITIVE]
15+
16+
str1 = Kernel.sprintf(params[:format], arg) # BAD
17+
str2 = sprintf(params[:format], arg) # BAD
18+
19+
str1 = Kernel.sprintf(params[:format]) # GOOD
20+
str2 = sprintf(params[:format]) # GOOD
21+
22+
stdout = IO.new 1
23+
stdout.printf(params[:format], arg) # BAD
24+
25+
stdout.printf(params[:format]) # GOOD
26+
27+
# Taint via string concatenation
28+
29+
printf("A log message: " + params[:format], arg) # BAD
30+
end
31+
end

0 commit comments

Comments
 (0)