Skip to content

Commit a2d4c20

Browse files
committed
C#: Fix FN by blocking flow out of the function call instead of out of the argument (which is incorrect when there is use-use flow).
1 parent 03e671a commit a2d4c20

File tree

3 files changed

+7
-2
lines changed

3 files changed

+7
-2
lines changed

csharp/ql/lib/semmle/code/csharp/security/dataflow/TaintedPathQuery.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ private module TaintedPathConfig implements DataFlow::StateConfigSig {
7777
predicate isBarrier(DataFlow::Node node, FlowState state) { node.(Sanitizer).isBarrier(state) }
7878

7979
predicate isBarrierOut(DataFlow::Node node, FlowState state) {
80-
isAdditionalFlowStep(node, state, _, _)
80+
isAdditionalFlowStep(_, state, node, _)
8181
}
8282
}
8383

csharp/ql/test/query-tests/Security Features/CWE-022/TaintedPath/TaintedPath.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public void ProcessRequest(HttpContext ctx)
6363
}
6464

6565
// This test ensures that we can flow through `Path.GetFullPath` and still get a result.
66-
ctx.Response.Write(File.ReadAllText(path)); // BAD [MISSING]
66+
ctx.Response.Write(File.ReadAllText(path)); // BAD
6767
}
6868

6969
public bool IsReusable

csharp/ql/test/query-tests/Security Features/CWE-022/TaintedPath/TaintedPath.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,21 @@
66
| TaintedPath.cs:36:25:36:31 | access to local variable badPath | TaintedPath.cs:10:23:10:45 | access to property QueryString : NameValueCollection | TaintedPath.cs:36:25:36:31 | access to local variable badPath | This path depends on a $@. | TaintedPath.cs:10:23:10:45 | access to property QueryString | user-provided value |
77
| TaintedPath.cs:38:49:38:55 | access to local variable badPath | TaintedPath.cs:10:23:10:45 | access to property QueryString : NameValueCollection | TaintedPath.cs:38:49:38:55 | access to local variable badPath | This path depends on a $@. | TaintedPath.cs:10:23:10:45 | access to property QueryString | user-provided value |
88
| TaintedPath.cs:51:26:51:29 | access to local variable path | TaintedPath.cs:10:23:10:45 | access to property QueryString : NameValueCollection | TaintedPath.cs:51:26:51:29 | access to local variable path | This path depends on a $@. | TaintedPath.cs:10:23:10:45 | access to property QueryString | user-provided value |
9+
| TaintedPath.cs:66:45:66:48 | access to local variable path | TaintedPath.cs:10:23:10:45 | access to property QueryString : NameValueCollection | TaintedPath.cs:66:45:66:48 | access to local variable path | This path depends on a $@. | TaintedPath.cs:10:23:10:45 | access to property QueryString | user-provided value |
910
edges
1011
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:12:50:12:53 | access to local variable path | provenance | |
1112
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:17:51:17:54 | access to local variable path | provenance | |
1213
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:25:30:25:33 | access to local variable path | provenance | |
1314
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:31:30:31:33 | access to local variable path | provenance | |
1415
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:35:16:35:22 | access to local variable badPath : String | provenance | |
1516
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:51:26:51:29 | access to local variable path | provenance | |
17+
| TaintedPath.cs:10:16:10:19 | access to local variable path : String | TaintedPath.cs:59:44:59:47 | access to local variable path : String | provenance | |
1618
| TaintedPath.cs:10:23:10:45 | access to property QueryString : NameValueCollection | TaintedPath.cs:10:16:10:19 | access to local variable path : String | provenance | |
1719
| TaintedPath.cs:10:23:10:45 | access to property QueryString : NameValueCollection | TaintedPath.cs:10:23:10:53 | access to indexer : String | provenance | MaD:1 |
1820
| TaintedPath.cs:10:23:10:53 | access to indexer : String | TaintedPath.cs:10:16:10:19 | access to local variable path : String | provenance | |
1921
| TaintedPath.cs:35:16:35:22 | access to local variable badPath : String | TaintedPath.cs:36:25:36:31 | access to local variable badPath | provenance | |
2022
| TaintedPath.cs:35:16:35:22 | access to local variable badPath : String | TaintedPath.cs:38:49:38:55 | access to local variable badPath | provenance | |
23+
| TaintedPath.cs:59:44:59:47 | access to local variable path : String | TaintedPath.cs:66:45:66:48 | access to local variable path | provenance | |
2124
models
2225
| 1 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
2326
nodes
@@ -32,4 +35,6 @@ nodes
3235
| TaintedPath.cs:36:25:36:31 | access to local variable badPath | semmle.label | access to local variable badPath |
3336
| TaintedPath.cs:38:49:38:55 | access to local variable badPath | semmle.label | access to local variable badPath |
3437
| TaintedPath.cs:51:26:51:29 | access to local variable path | semmle.label | access to local variable path |
38+
| TaintedPath.cs:59:44:59:47 | access to local variable path : String | semmle.label | access to local variable path : String |
39+
| TaintedPath.cs:66:45:66:48 | access to local variable path | semmle.label | access to local variable path |
3540
subpaths

0 commit comments

Comments
 (0)