Skip to content

Commit 13b2a0c

Browse files
committed
C#: Add a copy of all experimental queries (as is).
1 parent 28317d5 commit 13b2a0c

File tree

51 files changed

+2545
-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.

51 files changed

+2545
-0
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
using System;
2+
using System.IO;
3+
using System.Web;
4+
using System.Net;
5+
6+
public class TaintedWebClientHandler : IHttpHandler
7+
{
8+
public void ProcessRequest(HttpContext ctx)
9+
{
10+
String url = ctx.Request.QueryString["domain"];
11+
12+
// BAD: This could read any file on the filesystem. (../../../../etc/passwd)
13+
using(WebClient client = new WebClient()) {
14+
ctx.Response.Write(client.DownloadString(url));
15+
}
16+
17+
// BAD: This could still read any file on the filesystem. (https://../../../../etc/passwd)
18+
if (url.StartsWith("https://")){
19+
using(WebClient client = new WebClient()) {
20+
ctx.Response.Write(client.DownloadString(url));
21+
}
22+
}
23+
24+
// GOOD: IsWellFormedUriString ensures that it is a valid URL
25+
if (Uri.IsWellFormedUriString(url, UriKind.Absolute)){
26+
using(WebClient client = new WebClient()) {
27+
ctx.Response.Write(client.DownloadString(url));
28+
}
29+
}
30+
}
31+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The WebClient class provides a variety of methods for data transmission and
7+
communication with a particular URI. Despite of the class' naming convention,
8+
the URI scheme can also identify local resources, not only remote ones. Tainted
9+
by user-supplied input, the URI can be leveraged to access resources available
10+
on the local file system, therefore leading to the disclosure of sensitive
11+
information. This can be trivially achieved by supplying path traversal
12+
sequences (../) followed by an existing directory or file path.</p>
13+
14+
<p>Sanitization of user-supplied URI values using the
15+
<code>StartsWith("https://")</code> method is deemed insufficient in preventing
16+
arbitrary file reads. This is due to the fact that .NET ignores the protocol
17+
handler (https in this case) in URIs like the following:
18+
"https://../../../../etc/passwd".</p>
19+
20+
</overview>
21+
<recommendation>
22+
23+
<p>Validate user input before using it to ensure that is a URI of an external
24+
resource and not a local one.
25+
Potential solutions:</p>
26+
27+
<ul>
28+
<li>Sanitize potentially tainted paths using
29+
<code>System.Uri.IsWellFormedUriString</code>.</li>
30+
</ul>
31+
32+
</recommendation>
33+
<example>
34+
35+
<p>In the first example, a domain name is read from a <code>HttpRequest</code>
36+
and then this domain is requested using the method <code>DownloadString</code>.
37+
However, a malicious user could enter a local path - for example,
38+
"../../../etc/passwd" instead of a domain.
39+
In the second example, it appears that the user is restricted to the HTTPS
40+
protocol handler. However, a malicious user could still enter a local path,
41+
since as explained above the protocol handler will be ignored by .net. For
42+
example, the string "https://../../../etc/passwd" will result in the code
43+
reading the file located at "/etc/passwd", which is the system's password file.
44+
This file would then be sent back to the user, giving them access to all the
45+
system's passwords.</p>
46+
47+
<sample src="TaintedWebClient.cs" />
48+
49+
</example>
50+
<references>
51+
52+
<li>
53+
OWASP:
54+
<a href="https://owasp.org/www-community/attacks/Path_Traversal">Path Traversal</a>.
55+
</li>
56+
57+
</references>
58+
</qhelp>
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Uncontrolled data used in a WebClient
3+
* @description The WebClient class allows developers to request resources,
4+
* accessing resources influenced by users can allow an attacker to access local files.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id cs/webclient-path-injection
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-099
12+
* external/cwe/cwe-023
13+
* external/cwe/cwe-036
14+
* external/cwe/cwe-073
15+
*/
16+
17+
import csharp
18+
import TaintedWebClientLib
19+
import TaintedWebClient::PathGraph
20+
21+
from TaintedWebClient::PathNode source, TaintedWebClient::PathNode sink
22+
where TaintedWebClient::flowPath(source, sink)
23+
select sink.getNode(), source, sink, "A method of WebClient depepends on a $@.", source.getNode(),
24+
"user-provided value"
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import csharp
2+
import semmle.code.csharp.frameworks.system.Net
3+
import semmle.code.csharp.frameworks.System
4+
import semmle.code.csharp.security.dataflow.flowsources.FlowSources
5+
import semmle.code.csharp.security.Sanitizers
6+
7+
//If this leaves experimental this should probably go in semmle.code.csharp.frameworks.system.Net
8+
/** The `System.Net.WebClient` class. */
9+
class SystemNetWebClientClass extends SystemNetClass {
10+
SystemNetWebClientClass() { this.hasName("WebClient") }
11+
12+
/** Gets the `DownloadString` method. */
13+
Method getDownloadStringMethod() { result = this.getAMethod("DownloadString") }
14+
}
15+
16+
//If this leaves experimental this should probably go in semmle.code.csharp.frameworks.System
17+
//Extend the already existent SystemUriClass to not touch the stdlib.
18+
/** The `System.Uri` class. */
19+
class SystemUriClassExtra extends SystemUriClass {
20+
/** Gets the `IsWellFormedUriString` method. */
21+
Method getIsWellFormedUriStringMethod() { result = this.getAMethod("IsWellFormedUriString") }
22+
}
23+
24+
//If this leaves experimental this should probably go in semmle.code.csharp.frameworks.system
25+
/**
26+
* A data flow source for uncontrolled data in path expression vulnerabilities.
27+
*/
28+
abstract class Source extends DataFlow::Node { }
29+
30+
/**
31+
* A data flow sink for uncontrolled data in path expression vulnerabilities.
32+
*/
33+
abstract class Sink extends DataFlow::ExprNode { }
34+
35+
/**
36+
* A sanitizer for uncontrolled data in path expression vulnerabilities.
37+
*/
38+
abstract class Sanitizer extends DataFlow::ExprNode { }
39+
40+
/**
41+
* A taint-tracking configuration for uncontrolled data in path expression vulnerabilities.
42+
*/
43+
private module TaintedWebClientConfig implements DataFlow::ConfigSig {
44+
predicate isSource(DataFlow::Node source) { source instanceof Source }
45+
46+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
47+
48+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
49+
}
50+
51+
/**
52+
* A taint-tracking module for uncontrolled data in path expression vulnerabilities.
53+
*/
54+
module TaintedWebClient = TaintTracking::Global<TaintedWebClientConfig>;
55+
56+
/**
57+
* DEPRECATED: Use `ThreatModelSource` instead.
58+
*
59+
* A source of remote user input.
60+
*/
61+
deprecated class RemoteSource extends DataFlow::Node instanceof RemoteFlowSource { }
62+
63+
/** A source supported by the current threat model. */
64+
class ThreatModelSource extends Source instanceof ActiveThreatModelSource { }
65+
66+
/**
67+
* A path argument to a `WebClient` method call that has an address argument.
68+
*/
69+
class WebClientSink extends Sink {
70+
WebClientSink() {
71+
exists(Method m | m = any(SystemNetWebClientClass f).getAMethod() |
72+
this.getExpr() = m.getACall().getArgumentForName("address")
73+
)
74+
}
75+
}
76+
77+
/**
78+
* A call to `System.Uri.IsWellFormedUriString` that is considered to sanitize the input.
79+
*/
80+
class RequestMapPathSanitizer extends Sanitizer {
81+
RequestMapPathSanitizer() {
82+
exists(Method m | m = any(SystemUriClassExtra uri).getIsWellFormedUriStringMethod() |
83+
this.getExpr() = m.getACall().getArgument(0)
84+
)
85+
}
86+
}
87+
88+
private class SimpleTypeSanitizer extends Sanitizer, SimpleTypeSanitizedExpr { }
89+
90+
private class GuidSanitizer extends Sanitizer, GuidSanitizedExpr { }
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Cookies without <code>HttpOnly</code> flag are accessible to JavaScript running in the same origin. In case of
9+
Cross-Site Scripting (XSS) vulnerability the cookie can be stolen by malicious script.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
Protect sensitive cookies, such as related to authentication, by setting <code>HttpOnly</code> to <code>true</code> to make
16+
them not accessible to JavaScript. In ASP.NET case it is also possible to set the attribute via <code>&lt;httpCookies&gt;</code> element
17+
of <code>web.config</code> with the attribute <code>httpOnlyCookies="true"</code>.
18+
</p>
19+
</recommendation>
20+
21+
<example>
22+
23+
<p>
24+
In the example below <code>Microsoft.AspNetCore.Http.CookieOptions.HttpOnly</code> is set to <code>true</code>.
25+
</p>
26+
27+
<sample src="httponlyflagcore.cs" />
28+
29+
<p>
30+
In the following example <code>CookiePolicyOptions</code> are set programmatically to configure defaults.
31+
</p>
32+
33+
<sample src="cookiepolicyoptions.cs" />
34+
35+
<p>
36+
In the example below <code>System.Web.HttpCookie.HttpOnly</code> is set to <code>true</code>.
37+
</p>
38+
39+
<sample src="httponlyflag.cs" />
40+
41+
</example>
42+
43+
<references>
44+
45+
<li><a href="https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions.httponly">CookieOptions.HttpOnly Property,</a></li>
46+
<li><a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header,</li>
47+
<li><a href="https://msdn.microsoft.com/en-us/library/system.web.httpcookie.httponly(v=vs.110).aspx">HttpCookie.HttpOnly Property,</a></li>
48+
<li><a href="https://msdn.microsoft.com/library/ms228262%28v=vs.100%29.aspx">httpCookies Element,</a></li>
49+
50+
</references>
51+
</qhelp>
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* @name 'HttpOnly' attribute is not set to true
3+
* @description Omitting the 'HttpOnly' attribute for security sensitive data allows
4+
* malicious JavaScript to steal it in case of XSS vulnerability. Always set
5+
* 'HttpOnly' to 'true' to authentication related cookie to make it
6+
* not accessible by JavaScript.
7+
* @kind problem
8+
* @problem.severity warning
9+
* @precision high
10+
* @id cs/web/cookie-httponly-not-set
11+
* @tags security
12+
* experimental
13+
* external/cwe/cwe-1004
14+
*/
15+
16+
import csharp
17+
import semmle.code.asp.WebConfig
18+
import semmle.code.csharp.frameworks.system.Web
19+
import semmle.code.csharp.frameworks.microsoft.AspNetCore
20+
import experimental.dataflow.flowsources.AuthCookie
21+
22+
from Expr httpOnlySink
23+
where
24+
exists(Assignment a, Expr val |
25+
httpOnlySink = a.getRValue() and
26+
val.getValue() = "false" and
27+
(
28+
exists(ObjectCreation oc |
29+
getAValueForProp(oc, a, "HttpOnly") = val and
30+
(
31+
oc.getType() instanceof SystemWebHttpCookie and
32+
isCookieWithSensitiveName(oc.getArgument(0))
33+
or
34+
exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse |
35+
oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
36+
iResponse.getAppendMethod() = mc.getTarget() and
37+
isCookieWithSensitiveName(mc.getArgument(0)) and
38+
// there is no callback `OnAppendCookie` that sets `HttpOnly` to true
39+
not OnAppendCookieHttpOnlyTracking::flowTo(_) and
40+
// Passed as third argument to `IResponseCookies.Append`
41+
exists(DataFlow::Node creation, DataFlow::Node append |
42+
CookieOptionsTracking::flow(creation, append) and
43+
creation.asExpr() = oc and
44+
append.asExpr() = mc.getArgument(2)
45+
)
46+
)
47+
)
48+
)
49+
or
50+
exists(PropertyWrite pw |
51+
(
52+
pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or
53+
pw.getProperty().getDeclaringType() instanceof
54+
MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions
55+
) and
56+
pw.getProperty().getName() = "HttpOnly" and
57+
a.getLValue() = pw and
58+
DataFlow::localExprFlow(val, a.getRValue())
59+
)
60+
)
61+
)
62+
or
63+
exists(Call c |
64+
httpOnlySink = c and
65+
(
66+
exists(MicrosoftAspNetCoreHttpResponseCookies iResponse, MethodCall mc |
67+
// default is not configured or is not set to `Always`
68+
not getAValueForCookiePolicyProp("HttpOnly").getValue() = "1" and
69+
// there is no callback `OnAppendCookie` that sets `HttpOnly` to true
70+
not OnAppendCookieHttpOnlyTracking::flowTo(_) and
71+
iResponse.getAppendMethod() = mc.getTarget() and
72+
isCookieWithSensitiveName(mc.getArgument(0)) and
73+
(
74+
// `HttpOnly` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set
75+
exists(ObjectCreation oc |
76+
oc = c and
77+
oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
78+
not isPropertySet(oc, "HttpOnly") and
79+
exists(DataFlow::Node creation |
80+
CookieOptionsTracking::flow(creation, _) and
81+
creation.asExpr() = oc
82+
)
83+
)
84+
or
85+
// IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default
86+
mc = c and
87+
mc.getNumberOfArguments() < 3
88+
)
89+
)
90+
or
91+
exists(ObjectCreation oc |
92+
oc = c and
93+
oc.getType() instanceof SystemWebHttpCookie and
94+
isCookieWithSensitiveName(oc.getArgument(0)) and
95+
// the property wasn't explicitly set, so a default value from config is used
96+
not isPropertySet(oc, "HttpOnly") and
97+
// the default in config is not set to `true`
98+
not exists(XmlElement element |
99+
element instanceof HttpCookiesElement and
100+
element.(HttpCookiesElement).isHttpOnlyCookies()
101+
)
102+
)
103+
)
104+
)
105+
select httpOnlySink, "Cookie attribute 'HttpOnly' is not set to true."
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
public class Startup
2+
{
3+
// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
4+
public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
5+
{
6+
app.UseCookiePolicy(new CookiePolicyOptions()
7+
{
8+
Secure = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always,
9+
HttpOnly = Microsoft.AspNetCore.CookiePolicy.HttpOnlyPolicy.Always
10+
});
11+
}
12+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class MyController : Controller
2+
{
3+
void Login()
4+
{
5+
var cookie = new System.Web.HttpCookie("cookieName") { HttpOnly = true };
6+
}
7+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class MyController : Controller
2+
{
3+
void Login()
4+
{
5+
var cookieOptions = new Microsoft.AspNetCore.Http.CookieOptions() { HttpOnly = true };
6+
Response.Cookies.Append("auth", "secret", cookieOptions);
7+
}
8+
}

0 commit comments

Comments
 (0)