Skip to content

Commit 3ed8a8f

Browse files
Bubby4jBubby4j
committed
C#: Reduce false positives in cs/web/missing-x-frame-options
1 parent eb3ddb8 commit 3ed8a8f

22 files changed

Lines changed: 309 additions & 45 deletions

‎csharp/ql/src/Security Features/CWE-451/MissingXFrameOptions.qhelp‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55

66
<overview>
77
<p>
8-
Web sites that do not specify the <code>X-Frame-Options</code> HTTP header may be vulnerable to UI
9-
redress attacks ("clickjacking"). In these attacks, the vulnerable site is loaded in a frame on
10-
an attacker-controlled site which uses opaque or transparent layers to trick the user into
8+
Web sites that do not restrict framing using the <code>X-Frame-Options</code> HTTP header or the
9+
<code>frame-ancestors</code> Content Security Policy directive may be vulnerable to UI redress
10+
attacks ("clickjacking"). In these attacks, the vulnerable site is loaded in a frame on an
11+
attacker-controlled site which uses opaque or transparent layers to trick the user into
1112
unintentionally clicking a button or link on the vulnerable site.
1213
</p>
1314

@@ -17,16 +18,24 @@ unintentionally clicking a button or link on the vulnerable site.
1718
<p>
1819
Set the <code>X-Frame-Options</code> HTTP header to <code>DENY</code>, to instruct web browsers to
1920
block attempts to load the site in a frame. Alternatively, if framing is needed in certain
20-
circumstances, specify <code>SAMEORIGIN</code> or <code>ALLOW FROM: ...</code> to limit the ability
21-
to frame the site to pages from the same origin, or from an allowed whitelist of trusted domains.
21+
circumstances, specify <code>SAMEORIGIN</code> to permit framing by the same origin. The
22+
<code>frame-ancestors</code> directive in an enforced <code>Content-Security-Policy</code> header
23+
provides a more flexible alternative. For example, use <code>frame-ancestors 'none'</code> to
24+
prevent all framing, or use its source list to specify which origins may embed the application.
2225
</p>
2326
<p>
24-
For ASP.NET web applications, the header may be specified either in the <code>Web.config</code>
25-
file, using the <code>&lt;customHeaders&gt;</code> tag, or within the source code of the
26-
application using the <code>HttpResponse.AddHeader</code> method. In general, prefer specifying the
27-
header in the <code>Web.config</code> file to ensure it is added to all requests. If adding it
28-
to the source code, ensure that it is added unconditionally to all requests. For example, add the
29-
header in the <code>Application_BeginRequest</code> method in the <code>global.asax</code> file.
27+
For ASP.NET Framework applications, the header may be specified either in the
28+
<code>Web.config</code> file, using the <code>&lt;customHeaders&gt;</code> tag, or within the source
29+
code of the application using the <code>HttpResponse.AddHeader</code> method. In general, prefer
30+
specifying the header in the <code>Web.config</code> file to ensure it is added to all requests. If
31+
adding it to the source code, ensure that it is added unconditionally to all requests. For example,
32+
add the header in the <code>Application_BeginRequest</code> method in the
33+
<code>global.asax</code> file.
34+
</p>
35+
<p>
36+
For ASP.NET Core applications, set the header on <code>HttpResponse.Headers</code>. This can be
37+
done using the header dictionary's indexer or its <code>Append</code>, <code>Add</code>, or
38+
<code>TryAdd</code> methods.
3039
</p>
3140

3241
</recommendation>
@@ -41,11 +50,17 @@ The following example shows how to specify the <code>X-Frame-Options</code> head
4150

4251
<p>
4352
This next example shows how to specify the <code>X-Frame-Options</code> header within the
44-
<code>global.asax</code> file for ASP.NET application:
53+
<code>global.asax</code> file for an ASP.NET application:
4554
</p>
4655

4756
<sample src="MissingXFrameOptions.cs" />
4857

58+
<p>
59+
The following ASP.NET Core example uses an enforced Content Security Policy to disallow framing:
60+
</p>
61+
62+
<sample src="MissingXFrameOptionsAspNetCore.cs" />
63+
4964
</example>
5065
<references>
5166

@@ -57,6 +72,10 @@ OWASP:
5772
Mozilla:
5873
<a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options">X-Frame-Options</a>
5974
</li>
75+
<li>
76+
Mozilla:
77+
<a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors">Content-Security-Policy: frame-ancestors</a>
78+
</li>
6079

6180
</references>
6281
</qhelp>

‎csharp/ql/src/Security Features/CWE-451/MissingXFrameOptions.ql‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/**
2-
* @name Missing X-Frame-Options HTTP header
3-
* @description If the 'X-Frame-Options' setting is not provided, a malicious user may be able to
4-
* overlay their own UI on top of the site by using an iframe.
2+
* @name Missing clickjacking protection
3+
* @description If neither the 'X-Frame-Options' header nor a Content Security Policy
4+
* 'frame-ancestors' directive is provided, a malicious user may be able to overlay
5+
* their own UI on top of the site by using an iframe.
56
* @kind problem
67
* @problem.severity error
78
* @security-severity 7.5
@@ -14,7 +15,7 @@
1415

1516
import csharp
1617
import semmle.code.asp.WebConfig
17-
import semmle.code.csharp.frameworks.system.Web
18+
import Security_Features.MissingXFrameOptionsLib
1819

1920
XmlElement getAWebConfigRoot(WebConfigXml webConfig) {
2021
result = webConfig.getARootElement()
@@ -28,9 +29,10 @@ XmlElement getAWebConfigRoot(WebConfigXml webConfig) {
2829
}
2930

3031
/**
31-
* Holds if the `Web.config` file `webConfig` adds an `X-Frame-Options` header.
32+
* Holds if the `Web.config` file `webConfig` adds an `X-Frame-Options` header or a
33+
* `Content-Security-Policy` header containing a `frame-ancestors` directive.
3234
*/
33-
predicate hasWebConfigXFrameOptions(WebConfigXml webConfig) {
35+
predicate hasWebConfigClickjackingProtection(WebConfigXml webConfig) {
3436
// Looking for an entry in `webConfig` that looks like this:
3537
// ```xml
3638
// <system.webServer>
@@ -42,29 +44,30 @@ predicate hasWebConfigXFrameOptions(WebConfigXml webConfig) {
4244
// </system.webServer>
4345
// ```
4446
// This can also be in a `location`
45-
getAWebConfigRoot(webConfig)
46-
.getAChild("system.webServer")
47-
.getAChild("httpProtocol")
48-
.getAChild("customHeaders")
49-
.getAChild("add")
50-
.getAttributeValue("name") = "X-Frame-Options"
47+
exists(XmlElement add, string name |
48+
add =
49+
getAWebConfigRoot(webConfig)
50+
.getAChild("system.webServer")
51+
.getAChild("httpProtocol")
52+
.getAChild("customHeaders")
53+
.getAChild("add") and
54+
name = add.getAttributeValue("name") and
55+
(
56+
isXFrameOptionsHeaderName(name)
57+
or
58+
isContentSecurityPolicyHeaderName(name) and
59+
containsFrameAncestorsDirective(add.getAttributeValue("value"))
60+
)
61+
)
5162
}
5263

5364
/**
54-
* Holds if there exists a call to `AddHeader` or `AppendHeader` adding the `X-Frame-Options`
55-
* header.
65+
* Holds if code configures a clickjacking protection response header.
5666
*/
57-
predicate hasCodeXFrameOptions() {
58-
exists(MethodCall call |
59-
call.getTarget() = any(SystemWebHttpResponseClass r).getAppendHeaderMethod() or
60-
call.getTarget() = any(SystemWebHttpResponseClass r).getAddHeaderMethod()
61-
|
62-
call.getArgumentForName("name").getValue() = "X-Frame-Options"
63-
)
64-
}
67+
predicate hasCodeClickjackingProtection() { exists(getAClickjackingHeaderWrite()) }
6568

6669
from WebConfigXml webConfig
6770
where
68-
not hasWebConfigXFrameOptions(webConfig) and
69-
not hasCodeXFrameOptions()
70-
select webConfig, "Configuration file is missing the X-Frame-Options setting."
71+
not hasWebConfigClickjackingProtection(webConfig) and
72+
not hasCodeClickjackingProtection()
73+
select webConfig, "Configuration file is missing clickjacking protection."

‎csharp/ql/src/Security Features/CWE-451/MissingXFrameOptionsAspNetCore.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
void Configure(IApplicationBuilder app)
2+
{
3+
app.Use(async (context, next) =>
4+
{
5+
context.Response.Headers["Content-Security-Policy"] = "frame-ancestors 'none'";
6+
await next();
7+
});
8+
}

‎csharp/ql/src/Security Features/MissingXFrameOptionsLib.qll‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/** Provides predicates for recognizing clickjacking-related response-header configuration. */
2+
3+
import csharp
4+
import semmle.code.csharp.dataflow.DataFlow
5+
import semmle.code.csharp.frameworks.microsoft.AspNetCore
6+
import semmle.code.csharp.frameworks.system.Web
7+
8+
/** Holds if `name` is the `X-Frame-Options` header name, ignoring case. */
9+
bindingset[name]
10+
predicate isXFrameOptionsHeaderName(string name) { name.toLowerCase() = "x-frame-options" }
11+
12+
/** Holds if `name` is the enforced `Content-Security-Policy` header name, ignoring case. */
13+
bindingset[name]
14+
predicate isContentSecurityPolicyHeaderName(string name) {
15+
name.toLowerCase() = "content-security-policy"
16+
}
17+
18+
/**
19+
* Holds if `value` contains a `frame-ancestors` directive at the start of a CSP policy or
20+
* after a directive or policy separator.
21+
*/
22+
bindingset[value]
23+
predicate containsFrameAncestorsDirective(string value) {
24+
value.regexpMatch("(?is)(^|.*[;,])\\s*frame-ancestors(\\s|;|$).*")
25+
}
26+
27+
private predicate isHeaderNamesField(Expr name, string fieldName) {
28+
exists(FieldAccess access |
29+
name.stripImplicit() = access and
30+
access.getTarget().hasFullyQualifiedName("Microsoft.Net.Http.Headers", "HeaderNames", fieldName)
31+
)
32+
}
33+
34+
private predicate isXFrameOptionsHeaderNameExpr(Expr name) {
35+
isXFrameOptionsHeaderName(name.stripImplicit().getValue())
36+
or
37+
isHeaderNamesField(name, "XFrameOptions")
38+
}
39+
40+
private predicate isContentSecurityPolicyHeaderNameExpr(Expr name) {
41+
isContentSecurityPolicyHeaderName(name.stripImplicit().getValue())
42+
or
43+
isHeaderNamesField(name, "ContentSecurityPolicy")
44+
}
45+
46+
private predicate containsFrameAncestorsDirectiveExpr(Expr value) {
47+
containsFrameAncestorsDirective(value.stripImplicit().getValue())
48+
}
49+
50+
private predicate isClickjackingHeader(Expr name, Expr value) {
51+
isXFrameOptionsHeaderNameExpr(name)
52+
or
53+
isContentSecurityPolicyHeaderNameExpr(name) and containsFrameAncestorsDirectiveExpr(value)
54+
}
55+
56+
private predicate isDirectResponseHeadersAccess(Expr expr) {
57+
exists(PropertyAccessExpr headers, MicrosoftAspNetCoreHttpHttpResponse response |
58+
expr.stripImplicit() = headers and headers.getProperty() = response.getHeadersProperty()
59+
)
60+
}
61+
62+
private predicate isResponseHeadersAccess(Expr expr) {
63+
exists(Expr directAccess |
64+
isDirectResponseHeadersAccess(directAccess) and
65+
DataFlow::localExprFlow(directAccess, expr.stripImplicit())
66+
)
67+
}
68+
69+
private Expr getHeaderDictionaryReceiver(MethodCall call) {
70+
result = call.getQualifier()
71+
or
72+
call.getTarget().isExtensionMethod() and
73+
result = call.getArgumentForParameter(call.getTarget().getParameter(0))
74+
}
75+
76+
private predicate isClickjackingHeaderCall(MethodCall call) {
77+
(
78+
call.getTarget() = any(SystemWebHttpResponseClass r).getAppendHeaderMethod() or
79+
call.getTarget() = any(SystemWebHttpResponseClass r).getAddHeaderMethod()
80+
) and
81+
isClickjackingHeader(call.getArgumentForName("name"), call.getArgumentForName("value"))
82+
or
83+
call.getTarget().hasUndecoratedName(["Append", "Add", "TryAdd"]) and
84+
isResponseHeadersAccess(getHeaderDictionaryReceiver(call)) and
85+
isClickjackingHeader(call.getArgumentForName("key"), call.getArgumentForName("value"))
86+
}
87+
88+
private predicate isClickjackingHeaderIndexerAssignment(AssignExpr assignment) {
89+
exists(IndexerCall indexer |
90+
assignment.getLeftOperand() = indexer and
91+
isResponseHeadersAccess(indexer.getQualifier()) and
92+
isClickjackingHeader(indexer.getArgument(0), assignment.getRightOperand())
93+
)
94+
}
95+
96+
private predicate isClickjackingNamedHeaderPropertyAssignment(AssignExpr assignment) {
97+
exists(PropertyAccessExpr header |
98+
assignment.getLeftOperand() = header and
99+
isResponseHeadersAccess(header.(QualifiableExpr).getQualifier()) and
100+
(
101+
header.getProperty().hasName("XFrameOptions")
102+
or
103+
header.getProperty().hasName("ContentSecurityPolicy") and
104+
containsFrameAncestorsDirectiveExpr(assignment.getRightOperand())
105+
)
106+
)
107+
}
108+
109+
/** Gets an expression that configures a clickjacking-related response header. */
110+
Expr getAClickjackingHeaderWrite() {
111+
result = any(MethodCall call | isClickjackingHeaderCall(call))
112+
or
113+
result =
114+
any(AssignExpr assignment |
115+
isClickjackingHeaderIndexerAssignment(assignment) or
116+
isClickjackingNamedHeaderPropertyAssignment(assignment)
117+
)
118+
}

‎csharp/ql/src/change-notes/2026-09-10-missing-x-frame-options.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cs/web/missing-x-frame-options` query now recognizes clickjacking protection configured
5+
through ASP.NET Core response headers and enforced Content Security Policy `frame-ancestors`
6+
directives.

‎csharp/ql/test/query-tests/Security Features/CWE-451/MissingXFrameOptions/CodeAddedHeader/MissingXFrameOptions.qlref‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1 @@
1-
query: Security Features/CWE-451/MissingXFrameOptions.ql
2-
postprocess: utils/test/InlineExpectationsTestQuery.ql
1+
Security Features/CWE-451/MissingXFrameOptions.ql

‎csharp/ql/test/query-tests/Security Features/CWE-451/MissingXFrameOptions/HeaderWrites/HeaderWrites.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
using System.Collections.Generic;
2+
using Microsoft.AspNetCore.Http;
3+
using Microsoft.Net.Http.Headers;
4+
using AspNetCoreHttpContext = Microsoft.AspNetCore.Http.HttpContext;
5+
6+
public class HeaderWrites
7+
{
8+
public void AspNetCoreResponseHeaders(AspNetCoreHttpContext context)
9+
{
10+
context.Response.Headers.Append(HeaderNames.XFrameOptions, "DENY"); // $ Alert
11+
context.Response.Headers.Add("x-frame-options", "SAMEORIGIN"); // $ Alert
12+
context.Response.Headers.TryAdd(
13+
HeaderNames.ContentSecurityPolicy,
14+
"default-src 'self'; FrAmE-AnCeStOrS 'none'"); // $ Alert
15+
16+
context.Response.Headers["X-Frame-Options"] = "DENY"; // $ Alert
17+
context.Response.Headers["Content-Security-Policy"] =
18+
"default-src 'self'; frame-ancestors 'none'"; // $ Alert
19+
20+
context.Response.Headers.XFrameOptions = "DENY"; // $ Alert
21+
context.Response.Headers.ContentSecurityPolicy =
22+
"default-src 'self'; frame-ancestors 'self'"; // $ Alert
23+
context.Response.Headers["Content-Security-Policy"] =
24+
"default-src 'self', frame-ancestors 'none'"; // $ Alert
25+
26+
IHeaderDictionary responseHeaders = context.Response.Headers;
27+
responseHeaders.Append("X-Frame-Options", "DENY"); // $ Alert
28+
}
29+
30+
public void IgnoredHeaderWrites(AspNetCoreHttpContext context)
31+
{
32+
context.Request.Headers["X-Frame-Options"] = "DENY";
33+
34+
IHeaderDictionary reassignedHeaders = context.Response.Headers;
35+
reassignedHeaders = context.Request.Headers;
36+
reassignedHeaders["X-Frame-Options"] = "DENY";
37+
38+
var standaloneHeaders = new HeaderDictionary();
39+
standaloneHeaders.Append("X-Frame-Options", "DENY");
40+
standaloneHeaders["Content-Security-Policy"] = "frame-ancestors 'none'";
41+
42+
context.Response.Headers["Content-Security-Policy-Report-Only"] =
43+
"frame-ancestors 'none'";
44+
context.Response.Headers.ContentSecurityPolicyReportOnly = "frame-ancestors 'none'";
45+
context.Response.Headers["X-Content-Security-Policy"] = "frame-ancestors 'none'";
46+
context.Response.Headers["Content-Security-Policy"] = "default-src 'self'";
47+
context.Response.Headers["Content-Security-Policy"] =
48+
"report-uri https://example.test/frame-ancestors";
49+
context.Response.Headers["Content-Security-Policy"] =
50+
"default-src 'self'; not-frame-ancestors 'none'";
51+
}
52+
}

‎csharp/ql/test/query-tests/Security Features/CWE-451/MissingXFrameOptions/HeaderWrites/HeaderWrites.expected‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
| HeaderWrites.cs:10:9:10:74 | call to method Append | A clickjacking-related response header is configured here. |
2+
| HeaderWrites.cs:11:9:11:69 | call to method Add | A clickjacking-related response header is configured here. |
3+
| HeaderWrites.cs:12:9:14:57 | call to method TryAdd<String,StringValues> | A clickjacking-related response header is configured here. |
4+
| HeaderWrites.cs:16:9:16:60 | ... = ... | A clickjacking-related response header is configured here. |
5+
| HeaderWrites.cs:17:9:18:56 | ... = ... | A clickjacking-related response header is configured here. |
6+
| HeaderWrites.cs:20:9:20:55 | ... = ... | A clickjacking-related response header is configured here. |
7+
| HeaderWrites.cs:21:9:22:56 | ... = ... | A clickjacking-related response header is configured here. |
8+
| HeaderWrites.cs:23:9:24:56 | ... = ... | A clickjacking-related response header is configured here. |
9+
| HeaderWrites.cs:27:9:27:57 | call to method Append | A clickjacking-related response header is configured here. |

‎csharp/ql/test/query-tests/Security Features/CWE-451/MissingXFrameOptions/HeaderWrites/HeaderWrites.ql‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @kind problem
3+
* @id cs/test/clickjacking-header-write
4+
* @problem.severity warning
5+
*/
6+
7+
import csharp
8+
import Security_Features.MissingXFrameOptionsLib
9+
10+
from Expr write
11+
where write = getAClickjackingHeaderWrite()
12+
select write, "A clickjacking-related response header is configured here."

‎csharp/ql/test/query-tests/Security Features/CWE-451/MissingXFrameOptions/HeaderWrites/HeaderWrites.qlref‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: HeaderWrites.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

0 commit comments

Comments
 (0)