Skip to content

Commit 8102282

Browse files
geoffw0geoffw0
authored
Merge pull request #19222 from geoffw0/sinkstats
Rust: Define queries more consistently and include all sinks in stats
2 parents bc92a99 + 471f02c commit 8102282

14 files changed

Lines changed: 151 additions & 96 deletions

‎rust/ql/lib/codeql/rust/security/CleartextLoggingExtensions.qll‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ module CleartextLogging {
3636
*/
3737
private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
3838

39-
/** A sink for logging from model data. */
40-
private class ModelsAsDataSinks extends Sink {
41-
ModelsAsDataSinks() { exists(string s | sinkNode(this, s) and s.matches("log-injection%")) }
39+
/**
40+
* A sink for logging from model data.
41+
*/
42+
private class ModelsAsDataSink extends Sink {
43+
ModelsAsDataSink() { exists(string s | sinkNode(this, s) and s.matches("log-injection%")) }
4244
}
4345
}

‎rust/ql/lib/codeql/rust/security/CleartextTransmissionExtensions.qll‎

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,32 +7,52 @@ private import codeql.util.Unit
77
private import rust
88
private import codeql.rust.dataflow.DataFlow
99
private import codeql.rust.dataflow.FlowSink
10+
private import codeql.rust.security.SensitiveData
11+
private import codeql.rust.Concepts
1012

1113
/**
12-
* A data flow sink for cleartext transmission vulnerabilities. That is,
13-
* a `DataFlow::Node` of something that is transmitted over a network.
14+
* Provides default sources, sinks and barriers for detecting cleartext transmission
15+
* vulnerabilities, as well as extension points for adding your own.
1416
*/
15-
abstract class CleartextTransmissionSink extends DataFlow::Node { }
17+
module CleartextTransmission {
18+
/**
19+
* A data flow source for cleartext transmission vulnerabilities.
20+
*/
21+
abstract class Source extends DataFlow::Node { }
1622

17-
/**
18-
* A barrier for cleartext transmission vulnerabilities.
19-
*/
20-
abstract class CleartextTransmissionBarrier extends DataFlow::Node { }
23+
/**
24+
* A data flow sink for cleartext transmission vulnerabilities. That is,
25+
* a `DataFlow::Node` of something that is transmitted over a network.
26+
*/
27+
abstract class Sink extends QuerySink::Range {
28+
override string getSinkType() { result = "CleartextTransmission" }
29+
}
2130

22-
/**
23-
* A unit class for adding additional flow steps.
24-
*/
25-
class CleartextTransmissionAdditionalFlowStep extends Unit {
2631
/**
27-
* Holds if the step from `node1` to `node2` should be considered a flow
28-
* step for paths related to cleartext transmission vulnerabilities.
32+
* A barrier for cleartext transmission vulnerabilities.
2933
*/
30-
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
31-
}
34+
abstract class Barrier extends DataFlow::Node { }
3235

33-
/**
34-
* A sink defined through MaD.
35-
*/
36-
private class MadCleartextTransmissionSink extends CleartextTransmissionSink {
37-
MadCleartextTransmissionSink() { sinkNode(this, "transmission") }
36+
/**
37+
* A unit class for adding additional flow steps.
38+
*/
39+
class AdditionalFlowStep extends Unit {
40+
/**
41+
* Holds if the step from `node1` to `node2` should be considered a flow
42+
* step for paths related to cleartext transmission vulnerabilities.
43+
*/
44+
abstract predicate step(DataFlow::Node nodeFrom, DataFlow::Node nodeTo);
45+
}
46+
47+
/**
48+
* Sensitive data, considered as a flow source.
49+
*/
50+
private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
51+
52+
/**
53+
* A sink defined through MaD.
54+
*/
55+
private class ModelsAsDataSink extends Sink {
56+
ModelsAsDataSink() { sinkNode(this, "transmission") }
57+
}
3858
}

‎rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ module SqlInjection {
5151
SqlExecutionAsSink() { this = any(SqlExecution e).getSql() }
5252
}
5353

54-
/** A sink for sql-injection from model data. */
55-
private class ModelsAsDataSinks extends Sink {
56-
ModelsAsDataSinks() { sinkNode(this, "sql-injection") }
54+
/**
55+
* A sink for sql-injection from model data.
56+
*/
57+
private class ModelsAsDataSink extends Sink {
58+
ModelsAsDataSink() { sinkNode(this, "sql-injection") }
5759
}
5860
}

‎rust/ql/lib/codeql/rust/security/TaintedPathExtensions.qll‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ module TaintedPath {
2323
/**
2424
* A data flow sink for path injection vulnerabilities.
2525
*/
26-
abstract class Sink extends DataFlow::Node { }
26+
abstract class Sink extends QuerySink::Range {
27+
override string getSinkType() { result = "TaintedPath" }
28+
}
2729

2830
/**
2931
* A barrier for path injection vulnerabilities.

‎rust/ql/lib/codeql/rust/security/WeakSensitiveDataHashingExtensions.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ module NormalHashFunction {
4444
* hashing. That is, a broken or weak hashing algorithm.
4545
*/
4646
abstract class Sink extends QuerySink::Range {
47+
override string getSinkType() { result = "WeakSensitiveDataHashing" }
48+
4749
/**
4850
* Gets the name of the weak hashing algorithm.
4951
*/
@@ -76,8 +78,6 @@ module NormalHashFunction {
7678
class WeakHashingOperationInputAsSink extends Sink {
7779
Cryptography::HashingAlgorithm algorithm;
7880

79-
override string getSinkType() { result = "WeakSensitiveDataHashing" }
80-
8181
WeakHashingOperationInputAsSink() {
8282
exists(Cryptography::CryptographicOperation operation |
8383
algorithm.isWeak() and

‎rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll‎

Lines changed: 67 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -11,60 +11,81 @@ private import codeql.rust.dataflow.FlowSink
1111
private import codeql.rust.Concepts
1212

1313
/**
14-
* A data flow sink for regular expression injection vulnerabilities.
14+
* Provides default sources, sinks and barriers for detecting regular expression
15+
* injection vulnerabilities, as well as extension points for adding your own.
1516
*/
16-
abstract class RegexInjectionSink extends QuerySink::Range {
17-
override string getSinkType() { result = "RegexInjection" }
18-
}
17+
module RegexInjection {
18+
/**
19+
* A data flow source for regular expression injection vulnerabilities.
20+
*/
21+
abstract class Source extends DataFlow::Node { }
1922

20-
/**
21-
* A barrier for regular expression injection vulnerabilities.
22-
*/
23-
abstract class RegexInjectionBarrier extends DataFlow::Node { }
23+
/**
24+
* A data flow sink for regular expression injection vulnerabilities.
25+
*/
26+
abstract class Sink extends QuerySink::Range {
27+
override string getSinkType() { result = "RegexInjection" }
28+
}
2429

25-
/** A sink for `a` in `Regex::new(a)` when `a` is not a literal. */
26-
private class NewRegexInjectionSink extends RegexInjectionSink {
27-
NewRegexInjectionSink() {
28-
exists(CallExprCfgNode call, PathExpr path |
29-
path = call.getFunction().getExpr() and
30-
path.getResolvedCrateOrigin() = "repo:https://github.com/rust-lang/regex:regex" and
31-
path.getResolvedPath() = "<crate::regex::string::Regex>::new" and
32-
this.asExpr() = call.getArgument(0) and
33-
not this.asExpr() instanceof LiteralExprCfgNode
34-
)
30+
/**
31+
* A barrier for regular expression injection vulnerabilities.
32+
*/
33+
abstract class Barrier extends DataFlow::Node { }
34+
35+
/**
36+
* A unit class for adding additional flow steps.
37+
*/
38+
class AdditionalFlowStep extends Unit {
39+
/**
40+
* Holds if the step from `node1` to `node2` should be considered a flow
41+
* step for paths related to regular expression injection vulnerabilities.
42+
*/
43+
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
3544
}
36-
}
3745

38-
private class MadRegexInjectionSink extends RegexInjectionSink {
39-
MadRegexInjectionSink() { sinkNode(this, "regex-use") }
40-
}
46+
/**
47+
* An active threat-model source, considered as a flow source.
48+
*/
49+
private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
4150

42-
/**
43-
* A unit class for adding additional flow steps.
44-
*/
45-
class RegexInjectionAdditionalFlowStep extends Unit {
4651
/**
47-
* Holds if the step from `node1` to `node2` should be considered a flow
48-
* step for paths related to regular expression injection vulnerabilities.
52+
* A sink for `a` in `Regex::new(a)` when `a` is not a literal.
4953
*/
50-
abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
51-
}
54+
private class NewSink extends Sink {
55+
NewSink() {
56+
exists(CallExprCfgNode call, PathExpr path |
57+
path = call.getFunction().getExpr() and
58+
path.getResolvedCrateOrigin() = "repo:https://github.com/rust-lang/regex:regex" and
59+
path.getResolvedPath() = "<crate::regex::string::Regex>::new" and
60+
this.asExpr() = call.getArgument(0) and
61+
not this.asExpr() instanceof LiteralExprCfgNode
62+
)
63+
}
64+
}
5265

53-
/**
54-
* An escape barrier for regular expression injection vulnerabilities.
55-
*/
56-
private class RegexInjectionDefaultBarrier extends RegexInjectionBarrier {
57-
RegexInjectionDefaultBarrier() {
58-
// A barrier is any call to a function named `escape`, in particular this
59-
// makes calls to `regex::escape` a barrier.
60-
this.asExpr()
61-
.getExpr()
62-
.(CallExpr)
63-
.getFunction()
64-
.(PathExpr)
65-
.getPath()
66-
.getSegment()
67-
.getIdentifier()
68-
.getText() = "escape"
66+
/**
67+
* A sink for regular expression injection from model data.
68+
*/
69+
private class ModelsAsDataSink extends Sink {
70+
ModelsAsDataSink() { sinkNode(this, "regex-use") }
71+
}
72+
73+
/**
74+
* An escape barrier for regular expression injection vulnerabilities.
75+
*/
76+
private class DefaultBarrier extends Barrier {
77+
DefaultBarrier() {
78+
// A barrier is any call to a function named `escape`, in particular this
79+
// makes calls to `regex::escape` a barrier.
80+
this.asExpr()
81+
.getExpr()
82+
.(CallExpr)
83+
.getFunction()
84+
.(PathExpr)
85+
.getPath()
86+
.getSegment()
87+
.getIdentifier()
88+
.getText() = "escape"
89+
}
6990
}
7091
}

‎rust/ql/src/queries/security/CWE-020/RegexInjection.ql‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,22 @@
1717
private import rust
1818
private import codeql.rust.dataflow.DataFlow
1919
private import codeql.rust.dataflow.TaintTracking
20-
private import codeql.rust.Concepts
2120
private import codeql.rust.security.regex.RegexInjectionExtensions
2221

2322
/**
2423
* A taint configuration for detecting regular expression injection vulnerabilities.
2524
*/
2625
module RegexInjectionConfig implements DataFlow::ConfigSig {
27-
predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
26+
import RegexInjection
2827

29-
predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink }
28+
predicate isSource(DataFlow::Node source) { source instanceof Source }
3029

31-
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof RegexInjectionBarrier }
30+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
31+
32+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
3233

3334
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
34-
any(RegexInjectionAdditionalFlowStep s).step(nodeFrom, nodeTo)
35+
any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
3536
}
3637
}
3738

‎rust/ql/src/queries/security/CWE-022/TaintedPath.ql‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@
1616

1717
import rust
1818
import codeql.rust.dataflow.DataFlow
19-
import codeql.rust.dataflow.internal.DataFlowImpl as DataflowImpl
2019
import codeql.rust.dataflow.TaintTracking
20+
import codeql.rust.dataflow.internal.DataFlowImpl as DataflowImpl
21+
import codeql.rust.Concepts
2122
import codeql.rust.security.TaintedPathExtensions
22-
import TaintedPathFlow::PathGraph
23-
private import codeql.rust.Concepts
2423

2524
newtype NormalizationState =
2625
/** A state signifying that the file path has not been normalized. */
@@ -84,6 +83,8 @@ module TaintedPathConfig implements DataFlow::StateConfigSig {
8483

8584
module TaintedPathFlow = TaintTracking::GlobalWithState<TaintedPathConfig>;
8685

86+
import TaintedPathFlow::PathGraph
87+
8788
from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
8889
where TaintedPathFlow::flowPath(source, sink)
8990
select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),

‎rust/ql/src/queries/security/CWE-089/SqlInjection.ql‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,24 @@ import rust
1414
import codeql.rust.dataflow.DataFlow
1515
import codeql.rust.dataflow.TaintTracking
1616
import codeql.rust.security.SqlInjectionExtensions
17-
import SqlInjectionFlow::PathGraph
1817

1918
/**
2019
* A taint configuration for tainted data that reaches a SQL sink.
2120
*/
2221
module SqlInjectionConfig implements DataFlow::ConfigSig {
23-
predicate isSource(DataFlow::Node node) { node instanceof SqlInjection::Source }
22+
import SqlInjection
23+
24+
predicate isSource(DataFlow::Node node) { node instanceof Source }
2425

25-
predicate isSink(DataFlow::Node node) { node instanceof SqlInjection::Sink }
26+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
2627

27-
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof SqlInjection::Barrier }
28+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
2829
}
2930

3031
module SqlInjectionFlow = TaintTracking::Global<SqlInjectionConfig>;
3132

33+
import SqlInjectionFlow::PathGraph
34+
3235
from SqlInjectionFlow::PathNode sourceNode, SqlInjectionFlow::PathNode sinkNode
3336
where SqlInjectionFlow::flowPath(sourceNode, sinkNode)
3437
select sinkNode.getNode(), sourceNode, sinkNode, "This query depends on a $@.",

‎rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
import rust
1515
import codeql.rust.dataflow.DataFlow
16-
import codeql.rust.security.SensitiveData
1716
import codeql.rust.dataflow.TaintTracking
1817
import codeql.rust.security.CleartextTransmissionExtensions
1918

@@ -22,14 +21,16 @@ import codeql.rust.security.CleartextTransmissionExtensions
2221
* transmitted over a network.
2322
*/
2423
module CleartextTransmissionConfig implements DataFlow::ConfigSig {
25-
predicate isSource(DataFlow::Node node) { node instanceof SensitiveData }
24+
import CleartextTransmission
2625

27-
predicate isSink(DataFlow::Node node) { node instanceof CleartextTransmissionSink }
26+
predicate isSource(DataFlow::Node node) { node instanceof Source }
2827

29-
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof CleartextTransmissionBarrier }
28+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
29+
30+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
3031

3132
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
32-
any(CleartextTransmissionAdditionalFlowStep s).step(nodeFrom, nodeTo)
33+
any(AdditionalFlowStep s).step(nodeFrom, nodeTo)
3334
}
3435

3536
predicate isBarrierIn(DataFlow::Node node) {

0 commit comments

Comments
 (0)