Skip to content

Commit 561b769

Browse files
Merge pull request github#14343 from joefarebrother/csharp-xss-flow-step
C#: Add flow steps for View calls refering to Razor pages
2 parents 4e0cca9 + befb1cc commit 561b769

File tree

74 files changed

+3176
-0
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+3176
-0
lines changed
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
namespace test;
2+
3+
using System.Net;
4+
using Microsoft.AspNetCore.Mvc;
5+
using Microsoft.AspNetCore.Mvc.Razor;
6+
7+
public class UserData
8+
{
9+
public string Name { get; set; }
10+
}
11+
12+
public class TestController : Controller {
13+
public IActionResult Test(UserData tainted1) {
14+
return View("Test", tainted1);
15+
}
16+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
@page
2+
3+
@model UserData
4+
5+
@if (Model != null)
6+
{
7+
<h3>Hello "@Html.Raw(Model.Name)"</h3>
8+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@using test
2+
3+
@addTagHelper *, Microsoft.AspNetCore.Mvc.TagHelpers
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
| Views/Test/Test.cshtml:7:27:7:36 | access to property Name | Controllers/TestController.cs:13:40:13:47 | tainted1 : UserData | Views/Test/Test.cshtml:7:27:7:36 | 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:40:13:47 | tainted1 : UserData | User-provided value |
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Cross-site scripting
3+
* @description Writing user input directly to a web page
4+
* allows for a cross-site scripting vulnerability.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 6.1
8+
* @precision high
9+
* @id cs/web/xss
10+
* @tags security
11+
* external/cwe/cwe-079
12+
* external/cwe/cwe-116
13+
*/
14+
15+
import csharp
16+
import semmle.code.csharp.security.dataflow.XSSQuery
17+
18+
// import PathGraph // exclude query predicates with output dependant on the absolute filepath the tests are run in
19+
from XssNode source, XssNode sink, string message
20+
where xssFlow(source, sink, message)
21+
select sink, source, sink, "$@ flows to here and " + message, source, "User-provided value"
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<Project Sdk="Microsoft.NET.Sdk.Web">
2+
3+
<PropertyGroup>
4+
<TargetFramework>net7.0</TargetFramework>
5+
<Nullable>enable</Nullable>
6+
<ImplicitUsings>enable</ImplicitUsings>
7+
</PropertyGroup>
8+
9+
</Project>
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import os
2+
from create_database_utils import *
3+
4+
5+
os.environ['CODEQL_EXTRACTOR_CSHARP_STANDALONE_EXTRACT_WEB_VIEWS'] = 'true'
6+
run_codeql_database_create(lang="csharp", extra_args=["--extractor-option=buildless=true", "--extractor-option=cil=false"])

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
Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
/** Provides definitions and flow steps related to Razor pages. */
2+
3+
private import csharp
4+
private import codeql.util.Unit
5+
private import codeql.util.FilePath
6+
private import semmle.code.csharp.frameworks.microsoft.AspNetCore
7+
8+
/** A call to the `View` method */
9+
private class ViewCall extends MethodCall {
10+
ViewCall() {
11+
this.getTarget().hasFullyQualifiedName("Microsoft.AspNetCore.Mvc", "Controller", "View")
12+
}
13+
14+
/** Gets the `name` argument to this call, if any. */
15+
string getNameArgument() {
16+
exists(StringLiteral lit |
17+
this.getTarget().getParameter(0).getType() instanceof StringType and
18+
DataFlow::localExprFlow(lit, this.getArgument(0)) and
19+
result = lit.getValue()
20+
)
21+
}
22+
23+
/** Gets the `model` argument to this call, if any. */
24+
Expr getModelArgument() {
25+
exists(int i | i in [0 .. 1] |
26+
this.getTarget().getParameter(i).getType() instanceof ObjectType and
27+
result = this.getArgument(i)
28+
)
29+
}
30+
31+
/** Gets the MVC action method that this call is made from, if any. */
32+
Method getActionMethod() {
33+
result = this.getEnclosingCallable() and
34+
result = this.getController().getAnActionMethod()
35+
}
36+
37+
/**
38+
* Gets the action name that this call refers to, if any.
39+
* This is either the name argument, or the name of the action method calling this if there is no name argument.
40+
*/
41+
string getActionName() {
42+
result = this.getNameArgument()
43+
or
44+
not exists(this.getNameArgument()) and
45+
result = this.getActionMethod().getName()
46+
}
47+
48+
/** Gets the MVC controller that this call is made from, if any. */
49+
MicrosoftAspNetCoreMvcController getController() {
50+
result = this.getEnclosingCallable().getDeclaringType()
51+
}
52+
53+
/** Gets the name of the MVC controller that this call is made from, if any. */
54+
string getControllerName() { result + "Controller" = this.getController().getName() }
55+
56+
/** Gets the name of the Area that the controller of this call belongs to, if any. */
57+
string getAreaName() {
58+
exists(Attribute attr |
59+
attr = this.getController().getAnAttribute() and
60+
attr.getType().hasFullyQualifiedName("Microsoft.AspNetCore.Mvc", "AreaAttribute") and
61+
result = attr.getArgument(0).(StringLiteral).getValue()
62+
)
63+
}
64+
65+
/** `result` is `true` if this call is from a controller that is an Area, and `false` otherwise. */
66+
boolean hasArea() { if exists(this.getAreaName()) then result = true else result = false }
67+
}
68+
69+
/** A compiler-generated Razor page from a `.cshtml` file. */
70+
class RazorViewClass extends Class {
71+
AssemblyAttribute attr;
72+
73+
RazorViewClass() {
74+
exists(Class baseClass | baseClass = this.getBaseClass().getUnboundDeclaration() |
75+
baseClass.hasFullyQualifiedName("Microsoft.AspNetCore.Mvc.Razor", "RazorPage`1")
76+
or
77+
baseClass.hasFullyQualifiedName("Microsoft.AspNetCore.Mvc.RazorPages", "Page")
78+
) and
79+
attr.getFile() = this.getFile() and
80+
attr.getType()
81+
.hasFullyQualifiedName("Microsoft.AspNetCore.Razor.Hosting", "RazorCompiledItemAttribute")
82+
}
83+
84+
/**
85+
* Gets the filepath of the source file that this class was generated from.
86+
*
87+
* This is an absolute path if the database was extracted in standalone mode,
88+
* and is relative to to application root (the directory containing the .csproj file) otherwise.
89+
*/
90+
string getSourceFilepath() { result = attr.getArgument(2).(StringLiteral).getValue() }
91+
}
92+
93+
/**
94+
* Gets a possible prefix to be applied to view search paths to locate a Razor page.
95+
* This may be empty (for the case that the generated Razor page files contain paths relative to the application root),
96+
* or the absolute path of the directory containing the .csproj file (for the case that standalone extraction is used and the generated files contain absolute paths).
97+
*/
98+
private string getARazorPathPrefix() {
99+
result = ""
100+
or
101+
exists(File csproj |
102+
csproj.getExtension() = "csproj" and
103+
// possibly prepend '/' to match Windows absolute paths starting with `C:/` with paths appearing in the Razor file in standalone mode starting with `/C:/`
104+
result = ["/", ""] + csproj.getParentContainer().getAbsolutePath()
105+
)
106+
}
107+
108+
private class ViewCallJumpNode extends DataFlow::NonLocalJumpNode {
109+
RazorViewClass rp;
110+
111+
ViewCallJumpNode() {
112+
exists(ViewCall vc |
113+
viewCallRefersToPage(vc, rp) and
114+
this.asExpr() = vc.getModelArgument()
115+
)
116+
}
117+
118+
override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
119+
preservesValue = true and
120+
exists(PropertyAccess modelProp |
121+
result.asExpr() = modelProp and
122+
modelProp.getTarget().hasName("Model") and
123+
modelProp.getEnclosingCallable().getDeclaringType() = rp
124+
)
125+
}
126+
}
127+
128+
private predicate viewCallRefersToPage(ViewCall vc, RazorViewClass rp) {
129+
viewCallRefersToPageAbsolute(vc, rp) or
130+
viewCallRefersToPageRelative(vc, rp)
131+
}
132+
133+
bindingset[path]
134+
private string stripTilde(string path) { result = path.regexpReplaceAll("^~/", "/") }
135+
136+
private predicate viewCallRefersToPageAbsolute(ViewCall vc, RazorViewClass rp) {
137+
getARazorPathPrefix() + ["/", ""] + stripTilde(vc.getNameArgument()) = rp.getSourceFilepath()
138+
}
139+
140+
private predicate viewCallRefersToPageRelative(ViewCall vc, RazorViewClass rp) {
141+
rp = min(int i, RazorViewClass rp2 | matchesViewCallWithIndex(vc, rp2, i) | rp2 order by i)
142+
}
143+
144+
private predicate matchesViewCallWithIndex(ViewCall vc, RazorViewClass rp, int i) {
145+
exists(RelativeViewCallFilepath fp |
146+
fp.hasViewCallWithIndex(vc, i) and
147+
getARazorPathPrefix() + fp.getNormalizedPath() = rp.getSourceFilepath()
148+
)
149+
}
150+
151+
/** Gets the `i`th template for view discovery. */
152+
private string getViewSearchTemplate(int i, boolean isArea) {
153+
i = 0 and result = "/Areas/{2}/Views/{1}/{0}.cshtml" and isArea = true
154+
or
155+
i = 1 and result = "/Areas/{2}/Views/Shared/{0}.cshtml" and isArea = true
156+
or
157+
i = 2 and result = "/Views/{1}/{0}.cshtml" and isArea = false
158+
or
159+
i = 3 and result = "/Views/Shared/{0}.cshtml" and isArea = [true, false]
160+
or
161+
i = 4 and result = "/Pages/Shared/{0}.cshtml" and isArea = true
162+
or
163+
i = 5 and result = getAViewSearchTemplateInCode(isArea)
164+
}
165+
166+
/** Gets an additional template used for view discovery defined in code. */
167+
private string getAViewSearchTemplateInCode(boolean isArea) {
168+
exists(StringLiteral str, MethodCall addCall |
169+
addCall.getTarget().hasName("Add") and
170+
DataFlow::localExprFlow(str, addCall.getArgument(0)) and
171+
addCall.getQualifier() = getAViewLocationList(isArea) and
172+
result = str.getValue()
173+
)
174+
}
175+
176+
/** Gets a list expression containing view search locations */
177+
private Expr getAViewLocationList(boolean isArea) {
178+
exists(string name |
179+
result
180+
.(PropertyRead)
181+
.getProperty()
182+
.hasFullyQualifiedName("Microsoft.AspNetCore.Mvc.Razor", "RazorViewEngineOptions", name)
183+
|
184+
name = "ViewLocationFormats" and isArea = false
185+
or
186+
name = "AreaViewLocationFormats" and isArea = true
187+
// PageViewLocationFormats and AreaPageViewLocationFormats are used for calls within a page rather than a controller
188+
)
189+
}
190+
191+
/** A filepath that should be searched for a View call. */
192+
private class RelativeViewCallFilepath extends NormalizableFilepath {
193+
ViewCall vc_;
194+
int idx_;
195+
196+
RelativeViewCallFilepath() {
197+
exists(string template, string sub2, string sub1, string sub0 |
198+
template = getViewSearchTemplate(idx_, vc_.hasArea())
199+
|
200+
(
201+
if template.matches("%{2}%")
202+
then sub2 = template.replaceAll("{2}", vc_.getAreaName())
203+
else sub2 = template
204+
) and
205+
(
206+
if template.matches("%{1}%")
207+
then sub1 = sub2.replaceAll("{1}", vc_.getControllerName())
208+
else sub1 = sub2
209+
) and
210+
sub0 = sub1.replaceAll("{0}", vc_.getActionName()) and
211+
this = stripTilde(sub0)
212+
)
213+
}
214+
215+
/** Holds if this string is the `idx`th path that will be searched for the `vc` call. */
216+
predicate hasViewCallWithIndex(ViewCall vc, int idx) { vc = vc_ and idx = idx_ }
217+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Modelled additional flow steps to track flow from a `View` call in an MVC controller to the corresponding Razor View (`.cshtml`) file, which may result in additional results for queries such as `cs/web/xss`.

0 commit comments

Comments
 (0)