Skip to content

Commit 7d11e44

Browse files
joefarebrotherjoefarebrother
committed
Adress reveiw comments - update tests to path-problem and support all this qualifiers
1 parent cdeac9b commit 7d11e44

File tree

4 files changed

+37
-8
lines changed

4 files changed

+37
-8
lines changed

‎csharp/ql/lib/semmle/code/csharp/frameworks/Razor.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,17 +251,17 @@ private MethodCall getAPageCall(PageModelClass pm) {
251251
["Page", "RedirectToPage"])
252252
}
253253

254-
private MethodCall getImplicitThisCallInVoidHandler(PageModelClass pm) {
254+
private MethodCall getThisCallInVoidHandler(PageModelClass pm) {
255255
result.getEnclosingCallable() = pm.getAHandlerMethod() and
256256
result.getEnclosingCallable().getReturnType() instanceof VoidType and
257-
result.hasImplicitThisQualifier()
257+
result.getQualifier() instanceof ThisAccess
258258
}
259259

260260
private class PageModelJumpNode extends DataFlow::NonLocalJumpNode {
261261
PageModelClass pm;
262262

263263
PageModelJumpNode() {
264-
this.asExpr() = [getAPageCall(pm), getImplicitThisCallInVoidHandler(pm)].getQualifier()
264+
this.asExpr() = [getAPageCall(pm), getThisCallInVoidHandler(pm)].getQualifier()
265265
}
266266

267267
override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {

‎csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/TestModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ class TestModel : PageModel {
1010
private string source() { return "x"; }
1111

1212
public async Task<IActionResult> OnPostAsync() {
13-
Name = source();
13+
this.Name = source();
1414
return Page();
1515
}
1616

‎csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/test.expected

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,25 @@
1-
| TestPage.cshtml:5:16:5:25 | access to property Name | TestModel.cs:13:16:13:23 | call to method source | TestPage.cshtml:5:16:5:25 | access to property Name | Xss |
2-
| TestPage.cshtml:5:16:5:25 | access to property Name | TestModel.cs:18:16:18:23 | call to method source | TestPage.cshtml:5:16:5:25 | access to property Name | Xss |
1+
edges
2+
| TestModel.cs:13:9:13:12 | [post] this access : TestModel [property Name] : String | TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String |
3+
| TestModel.cs:13:21:13:28 | call to method source : String | TestModel.cs:13:9:13:12 | [post] this access : TestModel [property Name] : String |
4+
| TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String | TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String |
5+
| TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String | TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String |
6+
| TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String | TestModel.cs:18:16:18:23 | this access : TestModel [property Name] : String |
7+
| TestModel.cs:18:16:18:23 | call to method source : String | TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String |
8+
| TestModel.cs:18:16:18:23 | this access : TestModel [property Name] : String | TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String |
9+
| TestModel.cs:18:16:18:23 | this access : TestModel [property Name] : String | TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String |
10+
| TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String | TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String |
11+
| TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String | TestPage.cshtml:5:16:5:25 | access to property Name |
12+
nodes
13+
| TestModel.cs:13:9:13:12 | [post] this access : TestModel [property Name] : String | semmle.label | [post] this access : TestModel [property Name] : String |
14+
| TestModel.cs:13:21:13:28 | call to method source : String | semmle.label | call to method source : String |
15+
| TestModel.cs:14:16:14:21 | this access : TestModel [property Name] : String | semmle.label | this access : TestModel [property Name] : String |
16+
| TestModel.cs:18:9:18:12 | [post] this access : TestModel [property Name] : String | semmle.label | [post] this access : TestModel [property Name] : String |
17+
| TestModel.cs:18:16:18:23 | call to method source : String | semmle.label | call to method source : String |
18+
| TestModel.cs:18:16:18:23 | this access : TestModel [property Name] : String | semmle.label | this access : TestModel [property Name] : String |
19+
| TestPage.cshtml.g.cs:63:35:63:48 | access to property Model : TestModel [property Name] : String | semmle.label | access to property Model : TestModel [property Name] : String |
20+
| TestPage.cshtml:5:16:5:20 | access to property Model : TestModel [property Name] : String | semmle.label | access to property Model : TestModel [property Name] : String |
21+
| TestPage.cshtml:5:16:5:25 | access to property Name | semmle.label | access to property Name |
22+
subpaths
23+
#select
24+
| TestPage.cshtml:5:16:5:25 | access to property Name | TestModel.cs:13:21:13:28 | call to method source : String | TestPage.cshtml:5:16:5:25 | access to property Name | Xss |
25+
| TestPage.cshtml:5:16:5:25 | access to property Name | TestModel.cs:18:16:18:23 | call to method source : String | TestPage.cshtml:5:16:5:25 | access to property Name | Xss |

‎csharp/ql/test/query-tests/Security Features/CWE-079/XssPageModels/test.ql

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
15
import csharp
26
import semmle.code.csharp.security.dataflow.XSSQuery
37
import semmle.code.csharp.security.dataflow.XSSSinks
@@ -14,6 +18,8 @@ module TestXssTrackingConfig implements DataFlow::ConfigSig {
1418

1519
module TestXss = TaintTracking::Global<TestXssTrackingConfig>;
1620

17-
from DataFlow::Node source, DataFlow::Node sink
18-
where TestXss::flow(source, sink)
21+
import TestXss::PathGraph
22+
23+
from TestXss::PathNode source, TestXss::PathNode sink
24+
where TestXss::flowPath(source, sink)
1925
select sink, source, sink, "Xss"

0 commit comments

Comments
 (0)