Skip to content

Commit 711a715

Browse files
Sim4n6p-
Sim4n6
and
p-
authored
add CWE-770 experimental query for detection of DoS (#55)
* add CWE-770 experimental query for detection of DoS * Update ruby/src/security/CWE-770/DoS.qhelp * Rename DoS.ql to UserControlledMaxIterations.ql * Add UserControlledMaxIterations.qlref file for CWE-770 security testing * add some ruby files for the tests * Add UserControlledMaxIterations.expected file for CWE-770 security testing * Update ruby/src/security/CWE-770/UserControlledMaxIterations.ql * chore: Remove unused Ruby files for CWE-770 examples * added " provenance | |" to fix the expected test file --------- Co-authored-by: Peter Stöckli <[email protected]>
1 parent 316ed29 commit 711a715

File tree

6 files changed

+247
-0
lines changed

6 files changed

+247
-0
lines changed

‎ruby/src/security/CWE-770/UserControlledMaxIterations.qhelp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>When a remote user-controlled data value can be used as part of the limit of times an operation can be executed, such behavior could lead to a denial of service.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>Ensure the limitation and the validation of any incoming value to a reasonable value.</p>
13+
14+
</recommendation>
15+
16+
<example>
17+
<p>
18+
In this example a user-controlled data value such as `1_000` reaches a repeatable operation as `1_000` times. A simple exploit would be for an attacker to send a huge value as `999_999_999` or provoke an endless loop with a negative value.
19+
</p>
20+
21+
<sample src="examples/bad.rb" />
22+
23+
<p>To fix this vulnerability, it is required to constrain the size of the user input and validate the incoming value. </p>
24+
25+
<p>For illustration purposes, we can limit the possible values for the user input to between `1` and `1_000`.</p>
26+
27+
<sample src="examples/good.rb" />
28+
29+
</example>
30+
<references>
31+
32+
<li>
33+
<a href="https://nvd.nist.gov/vuln/detail/CVE-2022-23837">CVE-2022-23837: High severity denial of service vulnerability in Sidekiq, there is no limit on the number of days when requesting stats for the graph. This overloads the system, affecting the Web UI, and makes it unavailable to users.</a>
34+
</li>
35+
36+
<li><a href="https://github.com/sidekiq/sidekiq/commit/7785ac1399f1b28992adb56055f6acd88fd1d956">The suggested fix for the Sidekiq denial of service vulnerability.</a></li>
37+
38+
</references>
39+
</qhelp>

‎ruby/src/security/CWE-770/UserControlledMaxIterations.ql

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/**
2+
* @name Denial of Service using unconstrained integer/float value
3+
* @description A remote user-controlled integer/float value can reach a condition that controls how many times a repeatable operation can be executed. A malicious user may misuse that value to cause an application-level denial of service.
4+
* @kind path-problem
5+
* @id githubsecuritylab/user-controlled-max-iterations
6+
* @precision high
7+
* @problem.severity error
8+
* @tags security
9+
* experimental
10+
* external/cwe/cwe-770
11+
*/
12+
13+
import ruby
14+
import codeql.ruby.ApiGraphs
15+
import codeql.ruby.Concepts
16+
import codeql.ruby.TaintTracking
17+
import codeql.ruby.dataflow.RemoteFlowSources
18+
import codeql.ruby.dataflow.BarrierGuards
19+
import codeql.ruby.AST
20+
import codeql.ruby.controlflow.CfgNodes as CfgNodes
21+
import codeql.ruby.CFG
22+
import codeql.ruby.dataflow.internal.DataFlowPublic
23+
import codeql.ruby.InclusionTests
24+
25+
/**
26+
* Ensure that the user-provided value is limited to a certain amount
27+
*
28+
* @param node The node to check if limited by:
29+
* 1. A comparison operation node with a less than or less than or equal operator
30+
* 2. A comparison operation node with a greater than or greater than or equal operator
31+
* 3. A comparison operation node with a greater than or greater than or equal operator and the branch is false
32+
* 4. A comparison operation node with a less than or less than or equal operator and the branch is false
33+
*/
34+
predicate underAValue(CfgNodes::AstCfgNode g, CfgNode node, boolean branch) {
35+
exists(CfgNodes::ExprNodes::ComparisonOperationCfgNode cn | cn = g |
36+
exists(string op |
37+
(
38+
// arg <= LIMIT OR arg < LIMIT
39+
(op = "<" or op = "<=") and
40+
branch = true and
41+
op = cn.getOperator() and
42+
node = cn.getLeftOperand()
43+
or
44+
// LIMIT >= arg OR LIMIT > arg
45+
(op = ">" or op = ">=") and
46+
branch = true and
47+
op = cn.getOperator() and
48+
node = cn.getRightOperand()
49+
or
50+
// not arg >= LIMIT OR not arg > LIMIT
51+
(op = ">" or op = ">=") and
52+
branch = false and
53+
op = cn.getOperator() and
54+
node = cn.getLeftOperand()
55+
or
56+
// not LIMIT <= arg OR not LIMIT < argo
57+
(op = "<" or op = "<=") and
58+
branch = false and
59+
op = cn.getOperator() and
60+
node = cn.getRightOperand()
61+
)
62+
)
63+
)
64+
}
65+
66+
/**
67+
* Sidekiq ensure using the `params` function that all keys in the resulting hash are strings and ingest `request.params`. So a call to `params` function is considered as a remote flow source.
68+
*
69+
* https://github.com/sidekiq/sidekiq/blob/79d254d9045bb5805beed6aaffec1886ef89f71b/lib/sidekiq/web/action.rb#L30-L37
70+
*/
71+
class ParamsRFS extends RemoteFlowSource::Range {
72+
ParamsRFS() {
73+
exists(ElementReference er, MethodCall mc |
74+
er.getReceiver() = mc and
75+
mc.getMethodName() = "params" and
76+
this.asExpr() = er.getAControlFlowNode()
77+
)
78+
}
79+
80+
override string getSourceType() { result = "Request params data" }
81+
}
82+
83+
private module DoSConfig implements DataFlow::ConfigSig {
84+
predicate isSource(DataFlow::Node source) {
85+
//source instanceof ParamsRFS or
86+
source instanceof RemoteFlowSource
87+
}
88+
89+
predicate isBarrier(DataFlow::Node sanitizer) {
90+
// Sanitize the user-provided value if limited (underAValue check)
91+
sanitizer = DataFlow::BarrierGuard<underAValue/3>::getABarrierNode()
92+
}
93+
94+
/**
95+
* Support additional flow step for a case like using a default value `(params["days"] | 100).to_i`
96+
*/
97+
predicate isAdditionalFlowStep(DataFlow::Node previousNode, DataFlow::Node nextNode) {
98+
// Additional flow step like `(RFS | INTEGER).to_i` or `(FLOAT | RFS).to_f`
99+
exists(ParenthesizedExpr loe, DataFlow::CallNode c, ExprNode en |
100+
en = c.getReceiver() and
101+
c.getMethodName() = ["to_i", "to_f"] and
102+
c = nextNode and
103+
loe = en.asExpr().getExpr() and
104+
loe.getStmt(_).(LogicalOrExpr).getAnOperand() = previousNode.asExpr().getExpr() and
105+
not previousNode.asExpr().getExpr() instanceof IntegerLiteral
106+
)
107+
or
108+
// Additional flow step like `RFS.to_i` or `RFS.to_f`
109+
exists(MethodCall mc |
110+
mc.getReceiver() = previousNode.asExpr().getExpr() and
111+
mc.getMethodName() = ["to_i", "to_f"] and
112+
mc = nextNode.asExpr().getExpr()
113+
)
114+
}
115+
116+
/**
117+
* - Check if the user-provided value is used in a repeatable operation using `downto`, `upto` or `times`
118+
* - Check if the user-provided value is used in a repeatable operation using `for` loop or conditional loop like `until` or `while`.
119+
*/
120+
predicate isSink(DataFlow::Node sink) {
121+
// sink = n in `100.downto(n)` or `1.upto(n)`
122+
exists(MethodCall mc |
123+
sink.asExpr().getExpr() = mc.getArgument(0) and
124+
mc.getMethodName() = ["downto", "upto"]
125+
)
126+
or
127+
// sink = n in `n.times()` or `n.downto(1)` or `n.upto(100)`
128+
exists(MethodCall mc |
129+
sink.asExpr().getExpr() = mc.getReceiver() and
130+
mc.getMethodName() = ["times", "downto", "upto"]
131+
)
132+
or
133+
// sink = 1..n in `for i in 0..n`
134+
exists(ForExpr forLoop | sink.asExpr().getExpr() = forLoop.getValue())
135+
or
136+
// sink = n in `until n`
137+
exists(ConditionalLoop conditionalLoop |
138+
sink.asExpr().getExpr() = conditionalLoop.getCondition()
139+
)
140+
}
141+
}
142+
143+
module DoSFlow = TaintTracking::Global<DoSConfig>;
144+
145+
import DoSFlow::PathGraph
146+
147+
from DoSFlow::PathNode source, DoSFlow::PathNode sink
148+
where DoSFlow::flowPath(source, sink)
149+
select sink.getNode(), source, sink, "This $@ can control $@ a repeatable operation is executed.",
150+
source.getNode(), "user-provided value", sink.getNode(), "how many times"

‎ruby/test/security/CWE-770/UserControlledMaxIterations.expected

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
edges
2+
| UserControlledMaxIterations__bad.rb:3:7:3:11 | limit | UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | provenance | |
3+
| UserControlledMaxIterations__bad.rb:3:7:3:11 | limit | UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | provenance | |
4+
| UserControlledMaxIterations__bad.rb:3:15:3:20 | call to params | UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | provenance | |
5+
| UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | UserControlledMaxIterations__bad.rb:3:15:3:33 | call to to_i | provenance | |
6+
| UserControlledMaxIterations__bad.rb:3:15:3:33 | call to to_i | UserControlledMaxIterations__bad.rb:3:7:3:11 | limit | provenance | |
7+
nodes
8+
| UserControlledMaxIterations__bad.rb:3:7:3:11 | limit | semmle.label | limit |
9+
| UserControlledMaxIterations__bad.rb:3:15:3:20 | call to params | semmle.label | call to params |
10+
| UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | semmle.label | ...[...] |
11+
| UserControlledMaxIterations__bad.rb:3:15:3:33 | call to to_i | semmle.label | call to to_i |
12+
| UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | semmle.label | limit |
13+
| UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | semmle.label | limit |
14+
subpaths
15+
#select
16+
| UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | UserControlledMaxIterations__bad.rb:3:15:3:20 | call to params | UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | This $@ can control $@ a repeatable operation is executed. | UserControlledMaxIterations__bad.rb:3:15:3:20 | call to params | user-provided value | UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | how many times |
17+
| UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | This $@ can control $@ a repeatable operation is executed. | UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | user-provided value | UserControlledMaxIterations__bad.rb:11:7:11:11 | limit | how many times |
18+
| UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | UserControlledMaxIterations__bad.rb:3:15:3:20 | call to params | UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | This $@ can control $@ a repeatable operation is executed. | UserControlledMaxIterations__bad.rb:3:15:3:20 | call to params | user-provided value | UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | how many times |
19+
| UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | This $@ can control $@ a repeatable operation is executed. | UserControlledMaxIterations__bad.rb:3:15:3:28 | ...[...] | user-provided value | UserControlledMaxIterations__bad.rb:16:7:16:11 | limit | how many times |

‎ruby/test/security/CWE-770/UserControlledMaxIterations.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
security/CWE-770/UserControlledMaxIterations.ql

‎ruby/test/security/CWE-770/UserControlledMaxIterations__bad.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
class UserController < ActionController::Base
2+
def bad_examples
3+
limit = params[:limit].to_i
4+
5+
# repeat a simple operation for the number of limit specified using upto()
6+
1.upto(days) do |i|
7+
put "a repeatable operation"
8+
end
9+
10+
# repeat a simple operation for the number of limit specified using times()
11+
limit.times do
12+
put "a repeatable operation"
13+
end
14+
15+
# repeat a simple operation for the number of limit specified using downto()
16+
limit.downto(1) do |i|
17+
put "a repeatable operation"
18+
end
19+
20+
end
21+
end

‎ruby/test/security/CWE-770/UserControlledMaxIterations__good.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
class UserController < ActionController::Base
2+
def good_example
3+
limit = params[:limit].to_i
4+
5+
# limit the limit between 1 and 1000
6+
if not (limit => 1 && limit < 1000)
7+
limit = 10
8+
end
9+
10+
11+
# repeat a simple operation for the number of limit specified using upto()
12+
1.upto(days) do |i|
13+
put "a repeatable operation"
14+
end
15+
16+
end
17+
end

0 commit comments

Comments
 (0)