Skip to content

Commit 949b360

Browse files
committed
C#: Address review comments
1 parent 89e60dc commit 949b360

File tree

8 files changed

+36
-46
lines changed

8 files changed

+36
-46
lines changed

csharp/ql/src/Security Features/CWE-798/HardcodedCredentials.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ from
2121
where
2222
source = sourcePath.getNode() and
2323
sink = sinkPath.getNode() and
24-
c.hasFlow(source, sink) and
24+
c.hasFlowPath(sourcePath, sinkPath) and
2525
// Print the source value if it's available
2626
if exists(source.asExpr().getValue())
2727
then value = "The hard-coded value \"" + source.asExpr().getValue() + "\""

csharp/ql/src/semmle/code/csharp/dataflow/CallContext.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class CallContext extends TCallContext {
3333
class EmptyCallContext extends CallContext, TEmptyCallContext {
3434
override string toString() { result = "<empty>" }
3535

36-
override Location getLocation() { result instanceof EmptyLocation }
36+
override EmptyLocation getLocation() { any() }
3737
}
3838

3939
/**

csharp/ql/src/semmle/code/csharp/dataflow/internal/ControlFlowReachability.qll

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,34 @@ import csharp
22
private import DataFlowPrivate
33
private import DataFlowPublic
44

5-
private ControlFlowElement getAScope(boolean exactScope) {
6-
exists(ControlFlowReachabilityConfiguration c |
7-
c.candidate(_, _, result, exactScope, _) or
8-
c.candidateDef(_, _, result, exactScope, _)
9-
)
5+
private class ControlFlowScope extends ControlFlowElement {
6+
private boolean exactScope;
7+
8+
ControlFlowScope() {
9+
exists(ControlFlowReachabilityConfiguration c |
10+
c.candidate(_, _, this, exactScope, _) or
11+
c.candidateDef(_, _, this, exactScope, _)
12+
)
13+
}
14+
15+
predicate isExact() { exactScope = true }
16+
17+
predicate isNonExact() { exactScope = false }
1018
}
1119

12-
private ControlFlowElement getANonExactScopeChild(ControlFlowElement scope) {
13-
scope = getAScope(false) and
20+
private ControlFlowElement getANonExactScopeChild(ControlFlowScope scope) {
21+
scope.isNonExact() and
1422
result = scope
1523
or
1624
result = getANonExactScopeChild(scope).getAChild()
1725
}
1826

1927
pragma[noinline]
20-
private ControlFlow::BasicBlock getABasicBlockInScope(ControlFlowElement scope, boolean exactScope) {
28+
private ControlFlow::BasicBlock getABasicBlockInScope(ControlFlowScope scope, boolean exactScope) {
2129
result.getANode().getElement() = getANonExactScopeChild(scope) and
2230
exactScope = false
2331
or
24-
scope = getAScope(true) and
32+
scope.isExact() and
2533
result.getANode().getElement() = scope and
2634
exactScope = true
2735
}

csharp/ql/src/semmle/code/csharp/exprs/Creation.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,11 @@ class AnonymousFunctionExpr extends Expr, Callable, @anonymous_function_expr {
380380
override string getName() { result = "<anonymous>" }
381381

382382
override Type getReturnType() {
383-
result = getType().(SystemLinqExpressions::DelegateExtType).getDelegateType().getReturnType()
383+
result = this
384+
.getType()
385+
.(SystemLinqExpressions::DelegateExtType)
386+
.getDelegateType()
387+
.getReturnType()
384388
}
385389

386390
override AnonymousFunctionExpr getSourceDeclaration() { result = this }

csharp/ql/src/semmle/code/csharp/security/dataflow/HardcodedCredentials.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,21 @@ module HardcodedCredentials {
5555
not any(ReturnedByMockObject mock).getAnArgument() = sink.asExpr()
5656
}
5757

58-
override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
59-
super.hasFlow(source, sink) and
58+
override predicate hasFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
59+
super.hasFlowPath(source, sink) and
6060
// Exclude hard-coded credentials in tests if they only flow to calls to methods with a name
6161
// like "Add*" "Create*" or "Update*". The rationale is that hard-coded credentials within
6262
// tests that are only used for creating or setting values within tests are unlikely to
6363
// represent credentials to some accessible system.
6464
not (
65-
source.asExpr().getFile() instanceof TestFile and
65+
source.getNode().asExpr().getFile() instanceof TestFile and
6666
exists(MethodCall createOrAddCall, string createOrAddMethodName |
6767
createOrAddMethodName.matches("Update%") or
6868
createOrAddMethodName.matches("Create%") or
6969
createOrAddMethodName.matches("Add%")
7070
|
7171
createOrAddCall.getTarget().hasName(createOrAddMethodName) and
72-
createOrAddCall.getAnArgument() = sink.asExpr()
72+
createOrAddCall.getAnArgument() = sink.getNode().asExpr()
7373
)
7474
)
7575
}

csharp/ql/src/semmle/code/csharp/security/dataflow/ReDoS.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ module ReDoS {
4040
// a sub class of `Sink`, as that results in bad aggregate
4141
// recursion. Therefore, we overestimate the sinks here
4242
// and make the restriction later by overriding
43-
// `hasFlow()` below.
43+
// `hasFlowPath()` below.
4444
sink.asExpr() = any(RegexOperation ro).getInput()
4545
}
4646

4747
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
4848

49-
override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
50-
super.hasFlow(source, sink) and
51-
(sink instanceof Sink or sink instanceof ExponentialRegexSink)
49+
override predicate hasFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
50+
super.hasFlowPath(source, sink) and
51+
(sink.getNode() instanceof Sink or sink.getNode() instanceof ExponentialRegexSink)
5252
}
5353
}
5454

csharp/ql/src/semmle/code/csharp/security/dataflow/XMLEntityInjection.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ module XMLEntityInjection {
3939
// in the charpred, as that results in bad aggregate
4040
// recursion. Therefore, we overestimate the sinks here
4141
// and make the restriction later by overriding
42-
// `hasFlow()` below.
42+
// `hasFlowPath()` below.
4343
this.getExpr() = any(MethodCall mc |
4444
mc.getTarget().hasQualifiedName("System.Xml.XmlReader.Create") or
4545
mc.getTarget().hasQualifiedName("System.Xml.XmlDocument.Load") or
@@ -75,9 +75,9 @@ module XMLEntityInjection {
7575

7676
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
7777

78-
override predicate hasFlow(DataFlow::Node source, DataFlow::Node sink) {
79-
super.hasFlow(source, sink) and
80-
exists(sink.(Sink).getReason())
78+
override predicate hasFlowPath(DataFlow::PathNode source, DataFlow::PathNode sink) {
79+
super.hasFlowPath(source, sink) and
80+
exists(sink.getNode().(Sink).getReason())
8181
}
8282
}
8383

csharp/ql/test/query-tests/Security Features/CWE-798/HardcodedCredentials.expected

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,32 +11,10 @@ edges
1111
| TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" |
1212
#select
1313
| HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | The hard-coded value "myPa55word" flows to $@ which is compared against $@. | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | "myPa55word" | HardcodedCredentials.cs:17:13:17:20 | access to local variable password | access to local variable password |
14-
| HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | The hard-coded value "myPa55word" flows to $@ which is compared against $@. | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | "myPa55word" | HardcodedCredentials.cs:17:13:17:20 | access to local variable password | access to local variable password |
15-
| HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | The hard-coded value "myPa55word" flows to $@ which is compared against $@. | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | "myPa55word" | HardcodedCredentials.cs:17:13:17:20 | access to local variable password | access to local variable password |
16-
| HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | The hard-coded value "myPa55word" flows to $@ which is compared against $@. | HardcodedCredentials.cs:17:25:17:36 | "myPa55word" | "myPa55word" | HardcodedCredentials.cs:17:13:17:20 | access to local variable password | access to local variable password |
17-
| HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | HardcodedCredentials.cs:33:19:33:28 | "username" | name | HardcodedCredentials.cs:31:31:45:13 | object creation of type MembershipUser | object creation of type MembershipUser |
18-
| HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | HardcodedCredentials.cs:33:19:33:28 | "username" | name | HardcodedCredentials.cs:31:31:45:13 | object creation of type MembershipUser | object creation of type MembershipUser |
19-
| HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | HardcodedCredentials.cs:33:19:33:28 | "username" | name | HardcodedCredentials.cs:31:31:45:13 | object creation of type MembershipUser | object creation of type MembershipUser |
2014
| HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | HardcodedCredentials.cs:33:19:33:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | HardcodedCredentials.cs:33:19:33:28 | "username" | name | HardcodedCredentials.cs:31:31:45:13 | object creation of type MembershipUser | object creation of type MembershipUser |
2115
| HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | The hard-coded value "myNewPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | newPassword | HardcodedCredentials.cs:47:9:47:54 | call to method ChangePassword | call to method ChangePassword |
22-
| HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | The hard-coded value "myNewPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | newPassword | HardcodedCredentials.cs:47:9:47:54 | call to method ChangePassword | call to method ChangePassword |
23-
| HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | The hard-coded value "myNewPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | newPassword | HardcodedCredentials.cs:47:9:47:54 | call to method ChangePassword | call to method ChangePassword |
24-
| HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | The hard-coded value "myNewPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:47:39:47:53 | "myNewPa55word" | newPassword | HardcodedCredentials.cs:47:9:47:54 | call to method ChangePassword | call to method ChangePassword |
2516
| HardcodedCredentials.cs:49:30:49:60 | array creation of type Byte[] | HardcodedCredentials.cs:49:30:49:60 | array creation of type Byte[] | HardcodedCredentials.cs:52:13:52:23 | access to local variable rawCertData | This hard-coded value flows to the $@ parameter in $@. | HardcodedCredentials.cs:52:13:52:23 | access to local variable rawCertData | rawData | HardcodedCredentials.cs:51:33:53:25 | object creation of type X509Certificate2 | object creation of type X509Certificate2 |
26-
| HardcodedCredentials.cs:49:30:49:60 | array creation of type Byte[] | HardcodedCredentials.cs:49:30:49:60 | array creation of type Byte[] | HardcodedCredentials.cs:52:13:52:23 | access to local variable rawCertData | This hard-coded value flows to the $@ parameter in $@. | HardcodedCredentials.cs:52:13:52:23 | access to local variable rawCertData | rawData | HardcodedCredentials.cs:51:33:53:25 | object creation of type X509Certificate2 | object creation of type X509Certificate2 |
27-
| HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | The hard-coded value "myPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | password | HardcodedCredentials.cs:51:33:53:25 | object creation of type X509Certificate2 | object creation of type X509Certificate2 |
2817
| HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | The hard-coded value "myPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | password | HardcodedCredentials.cs:51:33:53:25 | object creation of type X509Certificate2 | object creation of type X509Certificate2 |
29-
| HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | The hard-coded value "myPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | password | HardcodedCredentials.cs:51:33:53:25 | object creation of type X509Certificate2 | object creation of type X509Certificate2 |
30-
| HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | The hard-coded value "myPa55word" flows to the $@ parameter in $@. | HardcodedCredentials.cs:53:13:53:24 | "myPa55word" | password | HardcodedCredentials.cs:51:33:53:25 | object creation of type X509Certificate2 | object creation of type X509Certificate2 |
31-
| HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | The hard-coded value "myusername" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:31:76:42 | "myusername" | username | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
3218
| HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | The hard-coded value "myusername" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:31:76:42 | "myusername" | username | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
33-
| HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | The hard-coded value "myusername" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:31:76:42 | "myusername" | username | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
34-
| HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | HardcodedCredentials.cs:76:31:76:42 | "myusername" | The hard-coded value "myusername" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:31:76:42 | "myusername" | username | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
35-
| HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | The hard-coded value "mypassword" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | password | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
3619
| HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | The hard-coded value "mypassword" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | password | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
37-
| HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | The hard-coded value "mypassword" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | password | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
38-
| HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | The hard-coded value "mypassword" flows to the $@ parameter in $@. | HardcodedCredentials.cs:76:45:76:56 | "mypassword" | password | HardcodedCredentials.cs:76:9:76:57 | call to method CreateUser | call to method CreateUser |
39-
| TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | TestHardcodedCredentials.cs:26:19:26:28 | "username" | name | TestHardcodedCredentials.cs:24:31:38:13 | object creation of type MembershipUser | object creation of type MembershipUser |
40-
| TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | TestHardcodedCredentials.cs:26:19:26:28 | "username" | name | TestHardcodedCredentials.cs:24:31:38:13 | object creation of type MembershipUser | object creation of type MembershipUser |
41-
| TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | TestHardcodedCredentials.cs:26:19:26:28 | "username" | name | TestHardcodedCredentials.cs:24:31:38:13 | object creation of type MembershipUser | object creation of type MembershipUser |
4220
| TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | TestHardcodedCredentials.cs:26:19:26:28 | "username" | The hard-coded value "username" flows to the $@ parameter in $@. | TestHardcodedCredentials.cs:26:19:26:28 | "username" | name | TestHardcodedCredentials.cs:24:31:38:13 | object creation of type MembershipUser | object creation of type MembershipUser |

0 commit comments

Comments
 (0)