-
Notifications
You must be signed in to change notification settings - Fork 20
add CWE-770 experimental query for detection of DoS #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
4cb2145
add CWE-770 experimental query for detection of DoS
Sim4n6 f9cd60d
Update ruby/src/security/CWE-770/DoS.qhelp
Sim4n6 f2258aa
Merge branch 'GitHubSecurityLab:main' into main
Sim4n6 d56a0bc
Rename DoS.ql to UserControlledMaxIterations.ql
Sim4n6 b06ea2b
Add UserControlledMaxIterations.qlref file for CWE-770 security testing
Sim4n6 7fa24a4
add some ruby files for the tests
Sim4n6 608efe8
Add UserControlledMaxIterations.expected file for CWE-770 security te…
Sim4n6 0b0d99b
Update ruby/src/security/CWE-770/UserControlledMaxIterations.ql
Sim4n6 7b24142
chore: Remove unused Ruby files for CWE-770 examples
Sim4n6 757cf05
added " provenance | |" to fix the expected test file
Sim4n6 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<!DOCTYPE qhelp PUBLIC | ||
"-//Semmle//qhelp//EN" | ||
"qhelp.dtd"> | ||
<qhelp> | ||
|
||
<overview> | ||
<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> | ||
|
||
</overview> | ||
<recommendation> | ||
|
||
<p>Ensure the limitation and the validation of any incoming value to a reasonable value.</p> | ||
|
||
</recommendation> | ||
|
||
<example> | ||
<p> | ||
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. | ||
</p> | ||
|
||
<sample src="examples/bad.rb" /> | ||
|
||
<p>To fix this vulnerability, it is required to constrain the size of the user input and validate the incoming value. </p> | ||
|
||
<p>For illustration pureposes, we can limit the possible values for the user input to between `1` and `1_000`.</p> | ||
|
||
<sample src="examples/good.rb" /> | ||
|
||
</example> | ||
<references> | ||
|
||
<li> | ||
<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> | ||
</li> | ||
|
||
<li><a href="https://github.com/sidekiq/sidekiq/commit/7785ac1399f1b28992adb56055f6acd88fd1d956">The suggested fix for the Sidekiq denial of service vulnerability.</a></li> | ||
|
||
</references> | ||
</qhelp> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
/** | ||
* @name Denial of Service using unconstrained integer/float value | ||
* @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. | ||
* @kind path-problem | ||
* @id rb/dos | ||
Sim4n6 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* @precision high | ||
* @problem.severity error | ||
* @tags security | ||
* experimental | ||
* external/cwe/cwe-770 | ||
*/ | ||
|
||
import ruby | ||
import codeql.ruby.ApiGraphs | ||
import codeql.ruby.Concepts | ||
import codeql.ruby.TaintTracking | ||
import codeql.ruby.dataflow.RemoteFlowSources | ||
import codeql.ruby.dataflow.BarrierGuards | ||
import codeql.ruby.AST | ||
import codeql.ruby.controlflow.CfgNodes as CfgNodes | ||
import codeql.ruby.CFG | ||
import codeql.ruby.dataflow.internal.DataFlowPublic | ||
import codeql.ruby.InclusionTests | ||
|
||
/** | ||
* Ensure that the user-provided value is limited to a certain amount | ||
* | ||
* @param node The node to check if limited by: | ||
* 1. A comparison operation node with a less than or less than or equal operator | ||
* 2. A comparison operation node with a greater than or greater than or equal operator | ||
* 3. A comparison operation node with a greater than or greater than or equal operator and the branch is false | ||
* 4. A comparison operation node with a less than or less than or equal operator and the branch is false | ||
*/ | ||
predicate underAValue(CfgNodes::AstCfgNode g, CfgNode node, boolean branch) { | ||
exists(CfgNodes::ExprNodes::ComparisonOperationCfgNode cn | cn = g | | ||
exists(string op | | ||
( | ||
// arg <= LIMIT OR arg < LIMIT | ||
(op = "<" or op = "<=") and | ||
branch = true and | ||
op = cn.getOperator() and | ||
node = cn.getLeftOperand() | ||
or | ||
// LIMIT >= arg OR LIMIT > arg | ||
(op = ">" or op = ">=") and | ||
branch = true and | ||
op = cn.getOperator() and | ||
node = cn.getRightOperand() | ||
or | ||
// not arg >= LIMIT OR not arg > LIMIT | ||
(op = ">" or op = ">=") and | ||
branch = false and | ||
op = cn.getOperator() and | ||
node = cn.getLeftOperand() | ||
or | ||
// not LIMIT <= arg OR not LIMIT < argo | ||
(op = "<" or op = "<=") and | ||
branch = false and | ||
op = cn.getOperator() and | ||
node = cn.getRightOperand() | ||
) | ||
) | ||
) | ||
} | ||
|
||
/** | ||
* 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. | ||
* | ||
* https://github.com/sidekiq/sidekiq/blob/79d254d9045bb5805beed6aaffec1886ef89f71b/lib/sidekiq/web/action.rb#L30-L37 | ||
*/ | ||
class ParamsRFS extends RemoteFlowSource::Range { | ||
ParamsRFS() { | ||
exists(ElementReference er, MethodCall mc | | ||
er.getReceiver() = mc and | ||
mc.getMethodName() = "params" and | ||
this.asExpr() = er.getAControlFlowNode() | ||
) | ||
} | ||
|
||
override string getSourceType() { result = "Request params data" } | ||
} | ||
|
||
private module DoSConfig implements DataFlow::ConfigSig { | ||
predicate isSource(DataFlow::Node source) { | ||
//source instanceof ParamsRFS or | ||
source instanceof RemoteFlowSource | ||
} | ||
|
||
predicate isBarrier(DataFlow::Node sanitizer) { | ||
// Sanitize the user-provided value if limited (underAValue check) | ||
sanitizer = DataFlow::BarrierGuard<underAValue/3>::getABarrierNode() | ||
} | ||
|
||
/** | ||
* Support additional flow step for a case like using a default value `(params["days"] | 100).to_i` | ||
*/ | ||
predicate isAdditionalFlowStep(DataFlow::Node previousNode, DataFlow::Node nextNode) { | ||
// Additional flow step like `(RFS | INTEGER).to_i` or `(FLOAT | RFS).to_f` | ||
exists(ParenthesizedExpr loe, DataFlow::CallNode c, ExprNode en | | ||
en = c.getReceiver() and | ||
c.getMethodName() = ["to_i", "to_f"] and | ||
c = nextNode and | ||
loe = en.asExpr().getExpr() and | ||
loe.getStmt(_).(LogicalOrExpr).getAnOperand() = previousNode.asExpr().getExpr() and | ||
not previousNode.asExpr().getExpr() instanceof IntegerLiteral | ||
) | ||
or | ||
// Additional flow step like `RFS.to_i` or `RFS.to_f` | ||
exists(MethodCall mc | | ||
mc.getReceiver() = previousNode.asExpr().getExpr() and | ||
mc.getMethodName() = ["to_i", "to_f"] and | ||
mc = nextNode.asExpr().getExpr() | ||
) | ||
} | ||
|
||
/** | ||
* - Check if the user-provided value is used in a repeatable operation using `downto`, `upto` or `times` | ||
* - Check if the user-provided value is used in a repeatable operation using `for` loop or conditional loop like `until` or `while`. | ||
*/ | ||
predicate isSink(DataFlow::Node sink) { | ||
// sink = n in `100.downto(n)` or `1.upto(n)` | ||
exists(MethodCall mc | | ||
sink.asExpr().getExpr() = mc.getArgument(0) and | ||
mc.getMethodName() = ["downto", "upto"] | ||
) | ||
or | ||
// sink = n in `n.times()` or `n.downto(1)` or `n.upto(100)` | ||
exists(MethodCall mc | | ||
sink.asExpr().getExpr() = mc.getReceiver() and | ||
mc.getMethodName() = ["times", "downto", "upto"] | ||
) | ||
or | ||
// sink = 1..n in `for i in 0..n` | ||
exists(ForExpr forLoop | sink.asExpr().getExpr() = forLoop.getValue()) | ||
or | ||
// sink = n in `until n` | ||
exists(ConditionalLoop conditionalLoop | | ||
sink.asExpr().getExpr() = conditionalLoop.getCondition() | ||
) | ||
} | ||
} | ||
|
||
module DoSFlow = TaintTracking::Global<DoSConfig>; | ||
|
||
import DoSFlow::PathGraph | ||
|
||
from DoSFlow::PathNode source, DoSFlow::PathNode sink | ||
where DoSFlow::flowPath(source, sink) | ||
select sink.getNode(), source, sink, "This $@ can control $@ a repeatable operation is executed.", | ||
source.getNode(), "user-provided value", sink.getNode(), "how many times" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
class UserController < ActionController::Base | ||
def bad_examples | ||
limit = params[:limit].to_i | ||
|
||
# repeat a simple operation for the number of limit specified using upto() | ||
1.upto(days) do |i| | ||
put "a repeatable operation" | ||
end | ||
|
||
# repeat a simple operation for the number of limit specified using times() | ||
limit.times do | ||
put "a repeatable operation" | ||
end | ||
|
||
# repeat a simple operation for the number of limit specified using downto() | ||
limit.downto(1) do |i| | ||
put "a repeatable operation" | ||
end | ||
|
||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
class UserController < ActionController::Base | ||
def good_example | ||
limit = params[:limit].to_i | ||
|
||
# limit the limit between 1 and 1000 | ||
if not (limit => 1 && limit < 1000) | ||
limit = 10 | ||
end | ||
|
||
|
||
# repeat a simple operation for the number of limit specified using upto() | ||
1.upto(days) do |i| | ||
put "a repeatable operation" | ||
end | ||
|
||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.