Skip to content

Commit c1cf2a1

Browse files
tamasvajktamasvajk
authored
Merge pull request github#5579 from edvraa/cookies
C#: HttpOnly and Secure cookie queries
2 parents 8bb47b9 + e790ee7 commit c1cf2a1

File tree

102 files changed

+2168
-23
lines changed

Some content is hidden

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

102 files changed

+2168
-23
lines changed

‎csharp/ql/src/Security Features/CWE-614/RequireSSL.ql

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,8 @@ import csharp
1717
import semmle.code.asp.WebConfig
1818
import semmle.code.csharp.frameworks.system.Web
1919

20-
class FormsElement extends XMLElement {
21-
FormsElement() {
22-
this = any(SystemWebXMLElement sw).getAChild("authentication").getAChild("forms")
23-
}
24-
25-
string getRequireSSL() { result = getAttribute("requireSSL").getValue().trim().toLowerCase() }
26-
27-
predicate isRequireSSL() { getRequireSSL() = "true" }
28-
}
29-
30-
class HttpCookiesElement extends XMLElement {
31-
HttpCookiesElement() { this = any(SystemWebXMLElement sw).getAChild("httpCookies") }
32-
33-
string getRequireSSL() { result = getAttribute("requireSSL").getValue().trim().toLowerCase() }
34-
35-
predicate isRequireSSL() {
36-
getRequireSSL() = "true"
37-
or
38-
not getRequireSSL() = "false" and
39-
exists(FormsElement forms | forms.getFile() = getFile() | forms.isRequireSSL())
40-
}
41-
}
42-
20+
// the query is a subset of `cs/web/cookie-secure-not-set` and
21+
// should be removed once it is promoted from experimental
4322
from XMLElement element
4423
where
4524
element instanceof FormsElement and

‎csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.qhelp

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>

‎csharp/ql/src/experimental/Security Features/CWE-1004/CookieWithoutHttpOnly.ql

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
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+
* external/cwe/cwe-1004
13+
*/
14+
15+
import csharp
16+
import semmle.code.asp.WebConfig
17+
import semmle.code.csharp.frameworks.system.Web
18+
import semmle.code.csharp.frameworks.microsoft.AspNetCore
19+
import experimental.dataflow.flowsources.AuthCookie
20+
21+
from Expr httpOnlySink
22+
where
23+
exists(Assignment a, Expr val |
24+
httpOnlySink = a.getRValue() and
25+
val.getValue() = "false" and
26+
(
27+
exists(ObjectCreation oc |
28+
getAValueForProp(oc, a, "HttpOnly") = val and
29+
(
30+
oc.getType() instanceof SystemWebHttpCookie and
31+
isCookieWithSensitiveName(oc.getArgument(0))
32+
or
33+
exists(MethodCall mc, MicrosoftAspNetCoreHttpResponseCookies iResponse |
34+
oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
35+
iResponse.getAppendMethod() = mc.getTarget() and
36+
isCookieWithSensitiveName(mc.getArgument(0)) and
37+
// there is no callback `OnAppendCookie` that sets `HttpOnly` to true
38+
not exists(
39+
OnAppendCookieHttpOnlyTrackingConfig config, DataFlow::Node source,
40+
DataFlow::Node sink
41+
|
42+
config.hasFlow(source, sink)
43+
) and
44+
// Passed as third argument to `IResponseCookies.Append`
45+
exists(
46+
CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation,
47+
DataFlow::Node append
48+
|
49+
cookieTracking.hasFlow(creation, append) and
50+
creation.asExpr() = oc and
51+
append.asExpr() = mc.getArgument(2)
52+
)
53+
)
54+
)
55+
)
56+
or
57+
exists(PropertyWrite pw |
58+
(
59+
pw.getProperty().getDeclaringType() instanceof MicrosoftAspNetCoreHttpCookieBuilder or
60+
pw.getProperty().getDeclaringType() instanceof
61+
MicrosoftAspNetCoreAuthenticationCookiesCookieAuthenticationOptions
62+
) and
63+
pw.getProperty().getName() = "HttpOnly" and
64+
a.getLValue() = pw and
65+
DataFlow::localExprFlow(val, a.getRValue())
66+
)
67+
)
68+
)
69+
or
70+
exists(Call c |
71+
httpOnlySink = c and
72+
(
73+
exists(MicrosoftAspNetCoreHttpResponseCookies iResponse, MethodCall mc |
74+
// default is not configured or is not set to `Always`
75+
not getAValueForCookiePolicyProp("HttpOnly").getValue() = "1" and
76+
// there is no callback `OnAppendCookie` that sets `HttpOnly` to true
77+
not exists(
78+
OnAppendCookieHttpOnlyTrackingConfig config, DataFlow::Node source, DataFlow::Node sink
79+
|
80+
config.hasFlow(source, sink)
81+
) and
82+
iResponse.getAppendMethod() = mc.getTarget() and
83+
isCookieWithSensitiveName(mc.getArgument(0)) and
84+
(
85+
// `HttpOnly` property in `CookieOptions` passed to IResponseCookies.Append(...) wasn't set
86+
exists(ObjectCreation oc |
87+
oc = c and
88+
oc.getType() instanceof MicrosoftAspNetCoreHttpCookieOptions and
89+
not isPropertySet(oc, "HttpOnly") and
90+
exists(
91+
CookieOptionsTrackingConfiguration cookieTracking, DataFlow::Node creation,
92+
DataFlow::Node append
93+
|
94+
cookieTracking.hasFlow(creation, append) and
95+
creation.asExpr() = oc
96+
)
97+
)
98+
or
99+
// IResponseCookies.Append(String, String) was called, `HttpOnly` is set to `false` by default
100+
mc = c and
101+
mc.getNumberOfArguments() < 3
102+
)
103+
)
104+
or
105+
exists(ObjectCreation oc |
106+
oc = c and
107+
oc.getType() instanceof SystemWebHttpCookie and
108+
isCookieWithSensitiveName(oc.getArgument(0)) and
109+
// the property wasn't explicitly set, so a default value from config is used
110+
not isPropertySet(oc, "HttpOnly") and
111+
// the default in config is not set to `true`
112+
not exists(XMLElement element |
113+
element instanceof HttpCookiesElement and
114+
element.(HttpCookiesElement).isHttpOnlyCookies()
115+
)
116+
)
117+
)
118+
)
119+
select httpOnlySink, "Cookie attribute 'HttpOnly' is not set to true."

‎csharp/ql/src/experimental/Security Features/CWE-1004/cookiepolicyoptions.cs

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+
}

‎csharp/ql/src/experimental/Security Features/CWE-1004/httponlyflag.cs

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+
}

‎csharp/ql/src/experimental/Security Features/CWE-1004/httponlyflagcore.cs

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+
}

‎csharp/ql/src/experimental/Security Features/CWE-614/CookieWithoutSecure.qhelp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Sensitive data that is transmitted using HTTP is vulnerable to being read by a third party. By default,
9+
cookies are sent via HTTP, not HTTPS.
10+
</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>
15+
In ASP.NET case when using cookies ensure that HTTPS is used by setting the property <code>Microsoft.AspNetCore.Http.CookieOptions.Secure</code> to <code>true</code>.
16+
</p>
17+
<p>
18+
In ASP.NET Core case when using cookies, ensure that HTTPS is used, either via the <code>&lt;forms&gt;</code> attribute above, or
19+
the <code>&lt;httpCookies&gt;</code> element, with the attribute <code>requireSSL="true"</code>. It is also possible to require cookies
20+
to use HTTPS programmatically, by setting the property <code>System.Web.HttpCookie.Secure</code> to <code>true</code>.
21+
</p>
22+
</recommendation>
23+
24+
<example>
25+
26+
<p>
27+
In the example below <code>Microsoft.AspNetCore.Http.CookieOptions.Secure</code> is set to <code>true</code> programmatically.
28+
</p>
29+
30+
<sample src="secureflagcore.cs" />
31+
32+
<p>
33+
In the following example <code>CookiePolicyOptions</code> are set programmatically to configure defaults.
34+
</p>
35+
36+
<sample src="cookiepolicyoptions.cs" />
37+
38+
<p>
39+
In the example below <code>System.Web.HttpCookie.Secure</code> is set to <code>true</code> programmatically.
40+
</p>
41+
42+
<sample src="secureflag.cs" />
43+
44+
</example>
45+
46+
<references>
47+
48+
<li><a href="https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.cookieoptions.secure">CookieOptions.Secure Property,</a></li>
49+
<li><a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie">Set-Cookie</a> Header,</li>
50+
<li><a href="https://msdn.microsoft.com/en-us/library/system.web.security.formsauthentication.requiressl(v=vs.110).aspx">FormsAuthentication.RequireSSL Property,</a></li>
51+
<li><a href="https://msdn.microsoft.com/en-us/library/1d3t3c61(v=vs.100).aspx">forms Element for authentication,</a></li>
52+
<li><a href="https://msdn.microsoft.com/library/ms228262%28v=vs.100%29.aspx">httpCookies Element,</a></li>
53+
54+
</references>
55+
</qhelp>

0 commit comments

Comments
 (0)