Skip to content

Commit 9af44ed

Browse files
joefarebrotherjoefarebrother
committed
Convert flow steps to value steps
1 parent 052166f commit 9af44ed

File tree

7 files changed

+116
-7
lines changed

7 files changed

+116
-7
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ private import semmle.code.csharp.controlflow.Guards
1515
private import semmle.code.csharp.dispatch.Dispatch
1616
private import semmle.code.csharp.frameworks.EntityFramework
1717
private import semmle.code.csharp.frameworks.NHibernate
18+
private import semmle.code.csharp.frameworks.Razor
1819
private import semmle.code.csharp.frameworks.system.Collections
1920
private import semmle.code.csharp.frameworks.system.threading.Tasks
2021
private import semmle.code.cil.Ssa::Ssa as CilSsa

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/TaintTrackingPrivate.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ private import dotnet
1212
// import `TaintedMember` and `AdditionalTaintStep` definitions from other files to avoid potential reevaluation
1313
private import semmle.code.csharp.frameworks.JsonNET
1414
private import semmle.code.csharp.frameworks.WCF
15-
private import semmle.code.csharp.frameworks.Razor
1615
private import semmle.code.csharp.security.dataflow.flowsources.Remote
1716

1817
/**

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,19 @@ class RazorPageClass extends Class {
8484
string getSourceFilepath() { result = attr.getArgument(2).(StringLiteral).getValue() }
8585
}
8686

87-
private class ViewCallFlowStep extends TaintTracking::AdditionalTaintStep {
88-
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
89-
exists(ViewCall vc, RazorPageClass rp, PropertyAccess modelProp |
90-
viewCallRefersToPage(vc, rp) and
91-
node1.asExpr() = vc.getModelArgument() and
92-
node2.asExpr() = modelProp and
87+
private class ViewCallJumpNode extends DataFlow::NonLocalJumpNode {
88+
ViewCall vc;
89+
RazorPageClass rp;
90+
91+
ViewCallJumpNode() {
92+
viewCallRefersToPage(vc, rp) and
93+
this.asExpr() = vc.getModelArgument()
94+
}
95+
96+
override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
97+
preservesValue = true and
98+
exists(PropertyAccess modelProp |
99+
result.asExpr() = modelProp and
93100
modelProp.getTarget().hasName("Model") and
94101
modelProp.getEnclosingCallable().getDeclaringType() = rp
95102
)

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSSRazorPages/Controllers/TestController.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,4 +145,11 @@ public IActionResult test22(UserData tainted22) {
145145
// Expected to find file /MyAreas/TestArea/Test4/Test22.cshtml
146146
return View("Test22", tainted22);
147147
}
148+
149+
public IActionResult test23(string tainted23) {
150+
// Expected to find file /Views/Shared/Test23.cshtml
151+
UserData x = new UserData();
152+
x.Name = tainted23;
153+
return View("Test23", x);
154+
}
148155
}

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSSRazorPages/Generated/Views_Shared_Test23.cshtml.g.cs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// A test file that mimics the output of compiling a `.cshtml` file
2+
// <auto-generated/>
3+
#pragma warning disable 1591
4+
[assembly: global::Microsoft.AspNetCore.Razor.Hosting.RazorCompiledItemAttribute(typeof(test.Views.Views_Shared_Test23), @"mvc.1.0.view", @"/Views/Shared/Test23.cshtml")]
5+
namespace test.Views
6+
{
7+
#line hidden
8+
using System;
9+
using System.Collections.Generic;
10+
using System.Linq;
11+
using System.Threading.Tasks;
12+
using Microsoft.AspNetCore.Mvc;
13+
using Microsoft.AspNetCore.Mvc.Rendering;
14+
using Microsoft.AspNetCore.Mvc.ViewFeatures;
15+
#nullable restore
16+
using test;
17+
18+
#line default
19+
#line hidden
20+
#nullable disable
21+
[global::Microsoft.AspNetCore.Razor.Hosting.RazorCompiledItemMetadataAttribute("Identifier", "/Views/Shared/Test23.cshtml")]
22+
public class Views_Shared_Test23 : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<UserData>
23+
{
24+
#pragma warning disable 1998
25+
public async override global::System.Threading.Tasks.Task ExecuteAsync()
26+
{
27+
#line 6 "Views/Shared/Test23.cshtml"
28+
if (Model != null)
29+
{
30+
31+
#line default
32+
#line hidden
33+
#nullable disable
34+
WriteLiteral(" <h3>Hello \"");
35+
#nullable restore
36+
#line 8 "Views/Shared/Test23.cshtml"
37+
Write(Html.Raw(Model.Name));
38+
39+
#line default
40+
#line hidden
41+
#nullable disable
42+
WriteLiteral("\"</h3>\n");
43+
#nullable restore
44+
#line 9 "Views/Shared/Test23.cshtml"
45+
}
46+
47+
#line default
48+
#line hidden
49+
#nullable disable
50+
}
51+
#pragma warning restore 1998
52+
#nullable restore
53+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
54+
public global::Microsoft.AspNetCore.Mvc.ViewFeatures.IModelExpressionProvider ModelExpressionProvider { get; private set; } = default!;
55+
#nullable disable
56+
#nullable restore
57+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
58+
public global::Microsoft.AspNetCore.Mvc.IUrlHelper Url { get; private set; } = default!;
59+
#nullable disable
60+
#nullable restore
61+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
62+
public global::Microsoft.AspNetCore.Mvc.IViewComponentHelper Component { get; private set; } = default!;
63+
#nullable disable
64+
#nullable restore
65+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
66+
public global::Microsoft.AspNetCore.Mvc.Rendering.IJsonHelper Json { get; private set; } = default!;
67+
#nullable disable
68+
#nullable restore
69+
[global::Microsoft.AspNetCore.Mvc.Razor.Internal.RazorInjectAttribute]
70+
public global::Microsoft.AspNetCore.Mvc.Rendering.IHtmlHelper<UserData> Html { get; private set; } = default!;
71+
#nullable disable
72+
}
73+
}
74+
#pragma warning restore 1591

‎csharp/ql/test/query-tests/Security Features/CWE-079/XSSRazorPages/Views/Shared/Test23.cshtml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@namespace test
2+
@model UserData
3+
@{
4+
}
5+
6+
@if (Model != null)
7+
{
8+
<h3>Hello "@Html.Raw(Model.Name)"</h3>
9+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ edges
4545
| Controllers/TestController.cs:131:31:131:39 | access to parameter tainted19 : UserData | Views/Shared/Test19.cshtml:8:16:8:20 | access to property Model : UserData |
4646
| Controllers/TestController.cs:139:42:139:50 | tainted21 : UserData | Controllers/TestController.cs:141:31:141:39 | access to parameter tainted21 : UserData |
4747
| Controllers/TestController.cs:141:31:141:39 | access to parameter tainted21 : UserData | Pages/Shared/Test21.cshtml:8:16:8:20 | access to property Model : UserData |
48+
| Controllers/TestController.cs:149:40:149:48 | tainted23 : String | Controllers/TestController.cs:152:18:152:26 | access to parameter tainted23 : String |
49+
| Controllers/TestController.cs:152:9:152:9 | [post] access to local variable x : UserData [property Name] : String | Controllers/TestController.cs:153:31:153:31 | access to local variable x : UserData [property Name] : String |
50+
| Controllers/TestController.cs:152:18:152:26 | access to parameter tainted23 : String | Controllers/TestController.cs:152:9:152:9 | [post] access to local variable x : UserData [property Name] : String |
51+
| Controllers/TestController.cs:153:31:153:31 | access to local variable x : UserData [property Name] : String | Views/Shared/Test23.cshtml:8:16:8:20 | access to property Model : UserData [property Name] : String |
4852
| Pages/Shared/Test21.cshtml:8:16:8:20 | access to property Model : UserData | Pages/Shared/Test21.cshtml:8:16:8:25 | access to property Name |
4953
| Views/Custom2/Test16.cshtml:8:16:8:20 | access to property Model : UserData | Views/Custom2/Test16.cshtml:8:16:8:25 | access to property Name |
5054
| Views/Custom/Test3/Test15.cshtml:8:16:8:20 | access to property Model : UserData | Views/Custom/Test3/Test15.cshtml:8:16:8:25 | access to property Name |
@@ -56,6 +60,7 @@ edges
5660
| Views/Shared/Test2.cshtml:8:16:8:20 | access to property Model : UserData | Views/Shared/Test2.cshtml:8:16:8:25 | access to property Name |
5761
| Views/Shared/Test14.cshtml:8:16:8:20 | access to property Model : UserData | Views/Shared/Test14.cshtml:8:16:8:25 | access to property Name |
5862
| Views/Shared/Test19.cshtml:8:16:8:20 | access to property Model : UserData | Views/Shared/Test19.cshtml:8:16:8:25 | access to property Name |
63+
| Views/Shared/Test23.cshtml:8:16:8:20 | access to property Model : UserData [property Name] : String | Views/Shared/Test23.cshtml:8:16:8:25 | access to property Name |
5964
| Views/Test2/Test10.cshtml:8:16:8:20 | access to property Model : UserData | Views/Test2/Test10.cshtml:8:16:8:25 | access to property Name |
6065
| Views/Test2/Test11.cshtml:8:16:8:20 | access to property Model : UserData | Views/Test2/Test11.cshtml:8:16:8:25 | access to property Name |
6166
| Views/Test/Test1.cshtml:8:16:8:20 | access to property Model : UserData | Views/Test/Test1.cshtml:8:16:8:25 | access to property Name |
@@ -111,6 +116,10 @@ nodes
111116
| Controllers/TestController.cs:131:31:131:39 | access to parameter tainted19 : UserData | semmle.label | access to parameter tainted19 : UserData |
112117
| Controllers/TestController.cs:139:42:139:50 | tainted21 : UserData | semmle.label | tainted21 : UserData |
113118
| Controllers/TestController.cs:141:31:141:39 | access to parameter tainted21 : UserData | semmle.label | access to parameter tainted21 : UserData |
119+
| Controllers/TestController.cs:149:40:149:48 | tainted23 : String | semmle.label | tainted23 : String |
120+
| Controllers/TestController.cs:152:9:152:9 | [post] access to local variable x : UserData [property Name] : String | semmle.label | [post] access to local variable x : UserData [property Name] : String |
121+
| Controllers/TestController.cs:152:18:152:26 | access to parameter tainted23 : String | semmle.label | access to parameter tainted23 : String |
122+
| Controllers/TestController.cs:153:31:153:31 | access to local variable x : UserData [property Name] : String | semmle.label | access to local variable x : UserData [property Name] : String |
114123
| Pages/Shared/Test21.cshtml:8:16:8:20 | access to property Model : UserData | semmle.label | access to property Model : UserData |
115124
| Pages/Shared/Test21.cshtml:8:16:8:25 | access to property Name | semmle.label | access to property Name |
116125
| Views/Custom2/Test16.cshtml:8:16:8:20 | access to property Model : UserData | semmle.label | access to property Model : UserData |
@@ -133,6 +142,8 @@ nodes
133142
| Views/Shared/Test14.cshtml:8:16:8:25 | access to property Name | semmle.label | access to property Name |
134143
| Views/Shared/Test19.cshtml:8:16:8:20 | access to property Model : UserData | semmle.label | access to property Model : UserData |
135144
| Views/Shared/Test19.cshtml:8:16:8:25 | access to property Name | semmle.label | access to property Name |
145+
| Views/Shared/Test23.cshtml:8:16:8:20 | access to property Model : UserData [property Name] : String | semmle.label | access to property Model : UserData [property Name] : String |
146+
| Views/Shared/Test23.cshtml:8:16:8:25 | access to property Name | semmle.label | access to property Name |
136147
| Views/Test2/Test10.cshtml:8:16:8:20 | access to property Model : UserData | semmle.label | access to property Model : UserData |
137148
| Views/Test2/Test10.cshtml:8:16:8:25 | access to property Name | semmle.label | access to property Name |
138149
| Views/Test2/Test11.cshtml:8:16:8:20 | access to property Model : UserData | semmle.label | access to property Model : UserData |
@@ -160,6 +171,7 @@ subpaths
160171
| Views/Shared/Test2.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:18:41:18:48 | tainted2 : UserData | Views/Shared/Test2.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:18:41:18:48 | tainted2 : UserData | User-provided value |
161172
| Views/Shared/Test14.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:86:42:86:50 | tainted14 : UserData | Views/Shared/Test14.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:86:42:86:50 | tainted14 : UserData | User-provided value |
162173
| Views/Shared/Test19.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:129:42:129:50 | tainted19 : UserData | Views/Shared/Test19.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:129:42:129:50 | tainted19 : UserData | User-provided value |
174+
| Views/Shared/Test23.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:149:40:149:48 | tainted23 : String | Views/Shared/Test23.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:149:40:149:48 | tainted23 : String | User-provided value |
163175
| Views/Test2/Test10.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:60:42:60:50 | tainted10 : UserData | Views/Test2/Test10.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:60:42:60:50 | tainted10 : UserData | User-provided value |
164176
| Views/Test2/Test11.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:65:42:65:50 | tainted11 : UserData | Views/Test2/Test11.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:65:42:65:50 | tainted11 : UserData | User-provided value |
165177
| Views/Test/Test1.cshtml:8:16:8:25 | access to property Name | Controllers/TestController.cs:13:41:13:48 | tainted1 : UserData | Views/Test/Test1.cshtml:8:16:8:25 | access to property Name | $@ flows to here and is written to HTML or JavaScript: Microsoft.AspNetCore.Mvc.ViewFeatures.HtmlHelper.Raw() method. | Controllers/TestController.cs:13:41:13:48 | tainted1 : UserData | User-provided value |

0 commit comments

Comments
 (0)