Skip to content

Commit dde37c6

Browse files
michaelnebelmichaelnebel
authored
Merge pull request github#12675 from michaelnebel/csharp/refactorflowapi
C#: Re-factor tainttracking configurations to use the new API.
2 parents 6275a01 + 31e352a commit dde37c6

13 files changed

+169
-30
lines changed

‎csharp/ql/lib/semmle/code/csharp/security/cryptography/EncryptionKeyDataFlowQuery.qll

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,11 @@ class SymmetricEncryptionCreateDecryptorSink extends SymmetricEncryptionKeySink
6464
}
6565

6666
/**
67+
* DEPRECATED: Use `SymmetricKey` instead.
68+
*
6769
* Symmetric Key Data Flow configuration.
6870
*/
69-
class SymmetricKeyTaintTrackingConfiguration extends TaintTracking::Configuration {
71+
deprecated class SymmetricKeyTaintTrackingConfiguration extends TaintTracking::Configuration {
7072
SymmetricKeyTaintTrackingConfiguration() { this = "SymmetricKeyTaintTracking" }
7173

7274
/** Holds if the node is a key source. */
@@ -78,3 +80,22 @@ class SymmetricKeyTaintTrackingConfiguration extends TaintTracking::Configuratio
7880
/** Holds if the node is a key sanitizer. */
7981
override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof KeySanitizer }
8082
}
83+
84+
/**
85+
* Symmetric Key Data Flow configuration.
86+
*/
87+
private module SymmetricKeyConfig implements DataFlow::ConfigSig {
88+
/** Holds if the node is a key source. */
89+
predicate isSource(DataFlow::Node src) { src instanceof KeySource }
90+
91+
/** Holds if the node is a symmetric encryption key sink. */
92+
predicate isSink(DataFlow::Node sink) { sink instanceof SymmetricEncryptionKeySink }
93+
94+
/** Holds if the node is a key sanitizer. */
95+
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof KeySanitizer }
96+
}
97+
98+
/**
99+
* Symmetric Key Data Flow configuration.
100+
*/
101+
module SymmetricKey = TaintTracking::Global<SymmetricKeyConfig>;

‎csharp/ql/lib/semmle/code/csharp/security/cryptography/HardcodedSymmetricEncryptionKey.qll

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ module HardcodedSymmetricEncryptionKey {
6262
}
6363

6464
/**
65+
* DEPRECATED: Use `HardCodedSymmetricEncryption` instead.
66+
*
6567
* A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
6668
*/
67-
class TaintTrackingConfiguration extends TaintTracking::Configuration {
69+
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
6870
TaintTrackingConfiguration() { this = "HardcodedSymmetricEncryptionKey" }
6971

7072
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -85,4 +87,32 @@ module HardcodedSymmetricEncryptionKey {
8587
)
8688
}
8789
}
90+
91+
/**
92+
* A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
93+
*/
94+
private module HardCodedSymmetricEncryptionConfig implements DataFlow::ConfigSig {
95+
predicate isSource(DataFlow::Node source) { source instanceof Source }
96+
97+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
98+
99+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
100+
101+
/**
102+
* Since `CryptographicBuffer` uses native code inside, taint tracking doesn't pass through it.
103+
* Need to create an additional custom step.
104+
*/
105+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
106+
exists(MethodCall mc, CryptographicBuffer c |
107+
pred.asExpr() = mc.getAnArgument() and
108+
mc.getTarget() = c.getAMethod() and
109+
succ.asExpr() = mc
110+
)
111+
}
112+
}
113+
114+
/**
115+
* A taint-tracking module for uncontrolled data in path expression vulnerabilities.
116+
*/
117+
module HardCodedSymmetricEncryption = TaintTracking::Global<HardCodedSymmetricEncryptionConfig>;
88118
}

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/CodeInjectionQuery.qll

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ abstract class Sink extends DataFlow::ExprNode { }
2525
abstract class Sanitizer extends DataFlow::ExprNode { }
2626

2727
/**
28+
* DEPRECATED: Use `CodeInjection` instead.
29+
*
2830
* A taint-tracking configuration for user input treated as code vulnerabilities.
2931
*/
30-
class TaintTrackingConfiguration extends TaintTracking::Configuration {
32+
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
3133
TaintTrackingConfiguration() { this = "CodeInjection" }
3234

3335
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -37,6 +39,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
3739
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
3840
}
3941

42+
/**
43+
* A taint-tracking configuration for user input treated as code vulnerabilities.
44+
*/
45+
private module CodeInjectionConfig implements DataFlow::ConfigSig {
46+
predicate isSource(DataFlow::Node source) { source instanceof Source }
47+
48+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
49+
50+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
51+
}
52+
53+
/**
54+
* A taint-tracking module for user input treated as code vulnerabilities.
55+
*/
56+
module CodeInjection = TaintTracking::Global<CodeInjectionConfig>;
57+
4058
/** A source of remote user input. */
4159
class RemoteSource extends Source instanceof RemoteFlowSource { }
4260

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/CommandInjectionQuery.qll

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ abstract class Sink extends DataFlow::ExprNode { }
2323
abstract class Sanitizer extends DataFlow::ExprNode { }
2424

2525
/**
26+
* DEPRECATED: Use `CommandInjection` instead.
27+
*
2628
* A taint-tracking configuration for command injection vulnerabilities.
2729
*/
28-
class TaintTrackingConfiguration extends TaintTracking::Configuration {
30+
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
2931
TaintTrackingConfiguration() { this = "CommandInjection" }
3032

3133
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -35,6 +37,32 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
3537
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
3638
}
3739

40+
/**
41+
* A taint-tracking configuration for command injection vulnerabilities.
42+
*/
43+
module CommandInjectionConfig implements DataFlow::ConfigSig {
44+
/**
45+
* Holds if `source` is a relevant data flow source.
46+
*/
47+
predicate isSource(DataFlow::Node source) { source instanceof Source }
48+
49+
/**
50+
* Holds if `sink` is a relevant data flow sink.
51+
*/
52+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
53+
54+
/**
55+
* Holds if data flow through `node` is prohibited. This completely removes
56+
* `node` from the data flow graph.
57+
*/
58+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
59+
}
60+
61+
/**
62+
* A taint-tracking module for command injection vulnerabilities.
63+
*/
64+
module CommandInjection = TaintTracking::Global<CommandInjectionConfig>;
65+
3866
/** A source of remote user input. */
3967
class RemoteSource extends Source instanceof RemoteFlowSource { }
4068

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/ConditionalBypassQuery.qll

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ abstract class Sink extends DataFlow::ExprNode {
3030
abstract class Sanitizer extends DataFlow::ExprNode { }
3131

3232
/**
33+
* DEPRECATED: Use `ConditionalBypass` instead.
34+
*
3335
* A taint-tracking configuration for user-controlled bypass of sensitive method.
3436
*/
35-
class Configuration extends TaintTracking::Configuration {
37+
deprecated class Configuration extends TaintTracking::Configuration {
3638
Configuration() { this = "UserControlledBypassOfSensitiveMethodConfiguration" }
3739

3840
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -42,6 +44,22 @@ class Configuration extends TaintTracking::Configuration {
4244
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
4345
}
4446

47+
/**
48+
* A taint-tracking configuration for user-controlled bypass of sensitive method.
49+
*/
50+
private module ConditionalBypassConfig implements DataFlow::ConfigSig {
51+
predicate isSource(DataFlow::Node source) { source instanceof Source }
52+
53+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
54+
55+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
56+
}
57+
58+
/**
59+
* A taint-tracking module for user-controlled bypass of sensitive method.
60+
*/
61+
module ConditionalBypass = TaintTracking::Global<ConditionalBypassConfig>;
62+
4563
/** A source of remote user input. */
4664
class RemoteSource extends Source instanceof RemoteFlowSource { }
4765

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/ExposureOfPrivateInformationQuery.qll

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ abstract class Sink extends DataFlow::ExprNode { }
2323
abstract class Sanitizer extends DataFlow::ExprNode { }
2424

2525
/**
26+
* DEPRECATED: Use `ExposureOfPrivateInformation` instead.
27+
*
2628
* A taint-tracking configuration for private information flowing unencrypted to an external location.
2729
*/
28-
class TaintTrackingConfiguration extends TaintTracking::Configuration {
30+
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
2931
TaintTrackingConfiguration() { this = "ExposureOfPrivateInformation" }
3032

3133
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -35,6 +37,22 @@ class TaintTrackingConfiguration extends TaintTracking::Configuration {
3537
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
3638
}
3739

40+
/**
41+
* A taint-tracking configuration for private information flowing unencrypted to an external location.
42+
*/
43+
private module ExposureOfPrivateInformationConfig implements DataFlow::ConfigSig {
44+
predicate isSource(DataFlow::Node source) { source instanceof Source }
45+
46+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
47+
48+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
49+
}
50+
51+
/**
52+
* A taint-tracking module for private information flowing unencrypted to an external location.
53+
*/
54+
module ExposureOfPrivateInformation = TaintTracking::Global<ExposureOfPrivateInformationConfig>;
55+
3856
private class PrivateDataSource extends Source {
3957
PrivateDataSource() { this.getExpr() instanceof PrivateDataExpr }
4058
}

‎csharp/ql/src/Security Features/CWE-078/CommandInjection.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515

1616
import csharp
1717
import semmle.code.csharp.security.dataflow.CommandInjectionQuery
18-
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
18+
import CommandInjection::PathGraph
1919

20-
from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
21-
where c.hasFlowPath(source, sink)
20+
from CommandInjection::PathNode source, CommandInjection::PathNode sink
21+
where CommandInjection::flowPath(source, sink)
2222
select sink.getNode(), source, sink, "This command line depends on a $@.", source.getNode(),
2323
"user-provided value"

‎csharp/ql/src/Security Features/CWE-078/StoredCommandInjection.ql

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,19 @@
1616
import csharp
1717
import semmle.code.csharp.security.dataflow.flowsources.Stored
1818
import semmle.code.csharp.security.dataflow.CommandInjectionQuery
19-
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
19+
import StoredCommandInjection::PathGraph
2020

21-
class StoredTaintTrackingConfiguration extends TaintTrackingConfiguration {
22-
override predicate isSource(DataFlow::Node source) { source instanceof StoredFlowSource }
21+
module StoredCommandInjectionConfig implements DataFlow::ConfigSig {
22+
predicate isSource(DataFlow::Node source) { source instanceof StoredFlowSource }
23+
24+
predicate isSink = CommandInjectionConfig::isSink/1;
25+
26+
predicate isBarrier = CommandInjectionConfig::isBarrier/1;
2327
}
2428

25-
from StoredTaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
26-
where c.hasFlowPath(source, sink)
29+
module StoredCommandInjection = TaintTracking::Global<StoredCommandInjectionConfig>;
30+
31+
from StoredCommandInjection::PathNode source, StoredCommandInjection::PathNode sink
32+
where StoredCommandInjection::flowPath(source, sink)
2733
select sink.getNode(), source, sink, "This command line depends on a $@.", source.getNode(),
2834
"stored (potentially user-provided) value"

‎csharp/ql/src/Security Features/CWE-094/CodeInjection.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515

1616
import csharp
1717
import semmle.code.csharp.security.dataflow.CodeInjectionQuery
18-
import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
18+
import CodeInjection::PathGraph
1919

20-
from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode sink
21-
where c.hasFlowPath(source, sink)
20+
from CodeInjection::PathNode source, CodeInjection::PathNode sink
21+
where CodeInjection::flowPath(source, sink)
2222
select sink.getNode(), source, sink, "This code compilation depends on a $@.", source.getNode(),
2323
"user-provided value"

‎csharp/ql/src/Security Features/CWE-321/HardcodedEncryptionKey.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
import csharp
1717
import semmle.code.csharp.security.cryptography.EncryptionKeyDataFlowQuery
18-
import DataFlow::PathGraph
18+
import SymmetricKey::PathGraph
1919

2020
/**
2121
* The creation of a literal byte array.
@@ -38,10 +38,10 @@ class StringLiteralSource extends KeySource {
3838
}
3939

4040
from
41-
SymmetricKeyTaintTrackingConfiguration keyFlow, DataFlow::PathNode source,
42-
DataFlow::PathNode sink, KeySource srcNode, SymmetricEncryptionKeySink sinkNode
41+
SymmetricKey::PathNode source, SymmetricKey::PathNode sink, KeySource srcNode,
42+
SymmetricEncryptionKeySink sinkNode
4343
where
44-
keyFlow.hasFlowPath(source, sink) and
44+
SymmetricKey::flowPath(source, sink) and
4545
source.getNode() = srcNode and
4646
sink.getNode() = sinkNode
4747
select sink.getNode(), source, sink,

0 commit comments

Comments
 (0)