mirror of
https://github.com/github/codeql.git
synced 2025-12-18 01:33:15 +01:00
Merge from main to resolve conflicts
This commit is contained in:
@@ -16,6 +16,8 @@ import python
|
||||
import semmle.python.dataflow.new.DataFlow
|
||||
import semmle.python.dataflow.new.TaintTracking
|
||||
import semmle.python.filters.Tests
|
||||
private import semmle.python.dataflow.new.internal.DataFlowDispatch as DataFlowDispatch
|
||||
private import semmle.python.dataflow.new.internal.Builtins::Builtins as Builtins
|
||||
|
||||
bindingset[char, fraction]
|
||||
predicate fewer_characters_than(StrConst str, string char, float fraction) {
|
||||
@@ -30,15 +32,13 @@ predicate fewer_characters_than(StrConst str, string char, float fraction) {
|
||||
}
|
||||
|
||||
predicate possible_reflective_name(string name) {
|
||||
exists(any(ModuleValue m).attr(name))
|
||||
any(Function f).getName() = name
|
||||
or
|
||||
exists(any(ClassValue c).lookup(name))
|
||||
any(Class c).getName() = name
|
||||
or
|
||||
any(ClassValue c).getName() = name
|
||||
any(Module m).getName() = name
|
||||
or
|
||||
exists(Module::named(name))
|
||||
or
|
||||
exists(Value::named(name))
|
||||
exists(Builtins::likelyBuiltin(name))
|
||||
}
|
||||
|
||||
int char_count(StrConst str) { result = count(string c | c = str.getText().charAt(_)) }
|
||||
@@ -84,7 +84,9 @@ class CredentialSink extends DataFlow::Node {
|
||||
name.regexpMatch(getACredentialRegex()) and
|
||||
not name.matches("%file")
|
||||
|
|
||||
any(FunctionValue func).getNamedArgumentForCall(_, name) = this.asCfgNode()
|
||||
exists(DataFlowDispatch::ArgumentPosition pos | pos.isKeyword(name) |
|
||||
this.(DataFlow::ArgumentNode).argumentOf(_, pos)
|
||||
)
|
||||
or
|
||||
exists(Keyword k | k.getArg() = name and k.getValue().getAFlowNode() = this.asCfgNode())
|
||||
or
|
||||
|
||||
38
python/ql/src/experimental/Security/CWE-770/UnicodeDoS.qhelp
Normal file
38
python/ql/src/experimental/Security/CWE-770/UnicodeDoS.qhelp
Normal file
@@ -0,0 +1,38 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>When a remote user-controlled data can reach a costly Unicode normalization with either form, NFKC or NFKD, an attack such as the One Million Unicode Characters, could lead to a denial of service on Windows OS.</p>
|
||||
|
||||
<p>And, with the use of special Unicode characters, like U+2100 (℀) or U+2105 (℅), the payload size could be tripled after the compatibility normalization.</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure limiting the size of any incoming data that would go through a costly operations, including a Windows Unicode normalization with NFKC or NFKD. Such a recommandation would avoid a potential denial of service.</p>
|
||||
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
In this example a simple user-controlled data reaches a Unicode normalization with the form "NFKC".
|
||||
</p>
|
||||
|
||||
<sample src="bad.py" />
|
||||
|
||||
<p>To fix this vulnerability, we need restrain the size of the user input.</p>
|
||||
|
||||
<p>For example, we can use the <code>len()</code> builtin function to limit the size of the user input.</p>
|
||||
|
||||
<sample src="good.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
<li>
|
||||
<a href="https://hackerone.com/reports/2258758">CVE-2023-46695: Potential denial of service vulnerability in Django UsernameField on Windows.</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
114
python/ql/src/experimental/Security/CWE-770/UnicodeDoS.ql
Normal file
114
python/ql/src/experimental/Security/CWE-770/UnicodeDoS.ql
Normal file
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* @name Denial of Service using Unicode Characters
|
||||
* @description A remote user-controlled data can reach a costly Unicode normalization with either form NFKC or NFKD. On Windows OS, with an attack such as the One Million Unicode Characters, this could lead to a denial of service. And, with the use of special Unicode characters, like U+2100 (℀) or U+2105 (℅), the payload size could be tripled.
|
||||
* @kind path-problem
|
||||
* @id py/unicode-dos
|
||||
* @precision high
|
||||
* @problem.severity error
|
||||
* @tags security
|
||||
* experimental
|
||||
* external/cwe/cwe-770
|
||||
*/
|
||||
|
||||
import python
|
||||
import semmle.python.ApiGraphs
|
||||
import semmle.python.Concepts
|
||||
import semmle.python.dataflow.new.TaintTracking
|
||||
import semmle.python.dataflow.new.internal.DataFlowPublic
|
||||
import semmle.python.dataflow.new.RemoteFlowSources
|
||||
|
||||
// The Unicode compatibility normalization calls from unicodedata, unidecode, pyunormalize
|
||||
// and textnorm modules. The use of argIdx is to constraint the argument being normalized.
|
||||
class UnicodeCompatibilityNormalize extends API::CallNode {
|
||||
int argIdx;
|
||||
|
||||
UnicodeCompatibilityNormalize() {
|
||||
(
|
||||
this = API::moduleImport("unicodedata").getMember("normalize").getACall() and
|
||||
this.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText() in ["NFKC", "NFKD"]
|
||||
or
|
||||
this = API::moduleImport("pyunormalize").getMember("normalize").getACall() and
|
||||
this.getParameter(0).getAValueReachingSink().asExpr().(StrConst).getText() in ["NFKC", "NFKD"]
|
||||
) and
|
||||
argIdx = 1
|
||||
or
|
||||
(
|
||||
this = API::moduleImport("textnorm").getMember("normalize_unicode").getACall() and
|
||||
this.getParameter(1).getAValueReachingSink().asExpr().(StrConst).getText() in ["NFKC", "NFKD"]
|
||||
or
|
||||
this = API::moduleImport("unidecode").getMember("unidecode").getACall()
|
||||
or
|
||||
this = API::moduleImport("pyunormalize").getMember(["NFKC", "NFKD"]).getACall()
|
||||
) and
|
||||
argIdx = 0
|
||||
}
|
||||
|
||||
DataFlow::Node getPathArg() { result = this.getArg(argIdx) }
|
||||
}
|
||||
|
||||
predicate underAValue(DataFlow::GuardNode g, ControlFlowNode node, boolean branch) {
|
||||
exists(CompareNode cn | cn = g |
|
||||
exists(API::CallNode lenCall, Cmpop op, Node n |
|
||||
lenCall = n.getALocalSource() and
|
||||
(
|
||||
// arg <= LIMIT OR arg < LIMIT
|
||||
(op instanceof LtE or op instanceof Lt) and
|
||||
branch = true and
|
||||
cn.operands(n.asCfgNode(), op, _)
|
||||
or
|
||||
// LIMIT >= arg OR LIMIT > arg
|
||||
(op instanceof GtE or op instanceof Gt) and
|
||||
branch = true and
|
||||
cn.operands(_, op, n.asCfgNode())
|
||||
or
|
||||
// not arg >= LIMIT OR not arg > LIMIT
|
||||
(op instanceof GtE or op instanceof Gt) and
|
||||
branch = false and
|
||||
cn.operands(n.asCfgNode(), op, _)
|
||||
or
|
||||
// not LIMIT <= arg OR not LIMIT < arg
|
||||
(op instanceof LtE or op instanceof Lt) and
|
||||
branch = false and
|
||||
cn.operands(_, op, n.asCfgNode())
|
||||
)
|
||||
|
|
||||
lenCall = API::builtin("len").getACall() and
|
||||
node = lenCall.getArg(0).asCfgNode()
|
||||
) //and
|
||||
//not cn.getLocation().getFile().inStdlib()
|
||||
)
|
||||
}
|
||||
|
||||
private module UnicodeDoSConfig implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
|
||||
|
||||
predicate isBarrier(DataFlow::Node sanitizer) {
|
||||
// underAValue is a check to ensure that the length of the user-provided value is limited to a certain amount
|
||||
sanitizer = DataFlow::BarrierGuard<underAValue/3>::getABarrierNode()
|
||||
}
|
||||
|
||||
predicate isSink(DataFlow::Node sink) {
|
||||
// Any call to the Unicode compatibility normalization is a costly operation
|
||||
sink = any(UnicodeCompatibilityNormalize ucn).getPathArg()
|
||||
or
|
||||
// The call to secure_filename() from pallets/werkzeug uses the Unicode compatibility normalization
|
||||
// under the hood, https://github.com/pallets/werkzeug/blob/d3dd65a27388fbd39d146caacf2563639ba622f0/src/werkzeug/utils.py#L218
|
||||
sink = API::moduleImport("werkzeug").getMember("secure_filename").getACall().getArg(_)
|
||||
or
|
||||
sink =
|
||||
API::moduleImport("werkzeug")
|
||||
.getMember("utils")
|
||||
.getMember("secure_filename")
|
||||
.getACall()
|
||||
.getArg(_)
|
||||
}
|
||||
}
|
||||
|
||||
module UnicodeDoSFlow = TaintTracking::Global<UnicodeDoSConfig>;
|
||||
|
||||
import UnicodeDoSFlow::PathGraph
|
||||
|
||||
from UnicodeDoSFlow::PathNode source, UnicodeDoSFlow::PathNode sink
|
||||
where UnicodeDoSFlow::flowPath(source, sink)
|
||||
select sink.getNode(), source, sink, "This $@ can reach a $@.", source.getNode(),
|
||||
"user-provided value", sink.getNode(), "costly Unicode normalization operation"
|
||||
17
python/ql/src/experimental/Security/CWE-770/bad.py
Normal file
17
python/ql/src/experimental/Security/CWE-770/bad.py
Normal file
@@ -0,0 +1,17 @@
|
||||
from flask import Flask, jsonify, request
|
||||
import unicodedata
|
||||
|
||||
app = Flask(__name__)
|
||||
|
||||
|
||||
@app.route("/bad_1")
|
||||
def bad_1():
|
||||
# User controlled data
|
||||
file_path = request.args.get("file_path", "")
|
||||
|
||||
# Normalize the file path using NFKC Unicode normalization
|
||||
return (
|
||||
unicodedata.normalize("NFKC", file_path),
|
||||
200,
|
||||
{"Content-Type": "application/octet-stream"},
|
||||
)
|
||||
16
python/ql/src/experimental/Security/CWE-770/good.py
Normal file
16
python/ql/src/experimental/Security/CWE-770/good.py
Normal file
@@ -0,0 +1,16 @@
|
||||
from flask import Flask, jsonify, request
|
||||
import unicodedata
|
||||
|
||||
app = Flask(__name__)
|
||||
|
||||
|
||||
@app.route("/good_1")
|
||||
def good_1():
|
||||
r = request.args.get("file_path", "")
|
||||
|
||||
if len(r) <= 1_000:
|
||||
# Normalize the r using NFKD Unicode normalization
|
||||
r = unicodedata.normalize("NFKD", r)
|
||||
return r, 200, {"Content-Type": "application/octet-stream"}
|
||||
else:
|
||||
return jsonify({"error": "File not found"}), 404
|
||||
Reference in New Issue
Block a user