Skip to content

Commit 1ec2698

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #22553 from Bubby4j/fix/cs-missing-x-frame-options
C#: Reduce false positives in cs/web/missing-x-frame-options
2 parents b4eb75e + 3d1eb19 commit 1ec2698

24 files changed

Lines changed: 328 additions & 45 deletions

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,16 @@ class MicrosoftNamespace extends Namespace {
99
this.hasName("Microsoft")
1010
}
1111
}
12+
13+
/** The `Microsoft.Net.Http.Headers.HeaderNames` class. */
14+
class MicrosoftNetHttpHeadersHeaderNames extends Class {
15+
MicrosoftNetHttpHeadersHeaderNames() {
16+
this.hasFullyQualifiedName("Microsoft.Net.Http.Headers", "HeaderNames")
17+
}
18+
19+
/** Gets the `XFrameOptions` field. */
20+
Field getXFrameOptionsField() { result = this.getField("XFrameOptions") }
21+
22+
/** Gets the `ContentSecurityPolicy` field. */
23+
Field getContentSecurityPolicyField() { result = this.getField("ContentSecurityPolicy") }
24+
}

‎csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/AspNetCore.qll‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,19 @@ class MicrosoftAspNetCoreHttpHeaderDictionaryExtensions extends RefType {
565565
Method getSetCommaSeparatedValuesMethod() { result = this.getAMethod("SetCommaSeparatedValues") }
566566
}
567567

568+
/** The `Microsoft.AspNetCore.Http.IHeaderDictionary` interface. */
569+
class MicrosoftAspNetCoreHttpIHeaderDictionary extends RefType {
570+
MicrosoftAspNetCoreHttpIHeaderDictionary() {
571+
this.hasFullyQualifiedName("Microsoft.AspNetCore.Http", "IHeaderDictionary")
572+
}
573+
574+
/** Gets the `XFrameOptions` property. */
575+
Property getXFrameOptionsProperty() { result = this.getProperty("XFrameOptions") }
576+
577+
/** Gets the `ContentSecurityPolicy` property. */
578+
Property getContentSecurityPolicyProperty() { result = this.getProperty("ContentSecurityPolicy") }
579+
}
580+
568581
/** The `Microsoft.AspNetCore.Http.CookieOptions` class. */
569582
class MicrosoftAspNetCoreHttpCookieOptions extends RefType {
570583
MicrosoftAspNetCoreHttpCookieOptions() {

‎csharp/ql/lib/semmle/code/csharp/security/MissingXFrameOptionsQuery.qll‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
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 isXFrameOptionsText(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 isContentSecurityPolicyText(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 isXFrameOptionsHeaderNameExpr(Expr name) {
28+
isXFrameOptionsText(name.stripImplicit().getValue())
29+
or
30+
name.stripImplicit().(FieldAccess).getTarget() =
31+
any(MicrosoftNetHttpHeadersHeaderNames f).getXFrameOptionsField()
32+
}
33+
34+
private predicate isContentSecurityPolicyHeaderNameExpr(Expr name) {
35+
isContentSecurityPolicyText(name.stripImplicit().getValue()) or
36+
name.stripImplicit().(FieldAccess).getTarget() =
37+
any(MicrosoftNetHttpHeadersHeaderNames f).getContentSecurityPolicyField()
38+
}
39+
40+
private predicate containsFrameAncestorsDirectiveExpr(Expr value) {
41+
containsFrameAncestorsDirective(value.stripImplicit().getValue())
42+
}
43+
44+
private predicate isDirectResponseHeadersAccess(Expr expr) {
45+
exists(PropertyAccess headers, MicrosoftAspNetCoreHttpHttpResponse response |
46+
expr.stripImplicit() = headers and headers.getProperty() = response.getHeadersProperty()
47+
)
48+
}
49+
50+
private predicate isCallOnResponseHeadersAccess(Call call) {
51+
exists(Expr qualifier |
52+
call.(MethodCall).getQualifier() = qualifier or
53+
call.(ExtensionMethodCall).getArgument(0) = qualifier or
54+
call.(AccessorCall).getQualifier() = qualifier
55+
|
56+
exists(Expr directAccess |
57+
isDirectResponseHeadersAccess(directAccess) and
58+
DataFlow::localExprFlow(directAccess, qualifier.stripImplicit())
59+
)
60+
)
61+
}
62+
63+
private predicate isClickjackingHeaderCall(MethodCall call) {
64+
(
65+
call.getTarget() = any(SystemWebHttpResponseClass r).getAppendHeaderMethod() or
66+
call.getTarget() = any(SystemWebHttpResponseClass r).getAddHeaderMethod()
67+
) and
68+
(
69+
isXFrameOptionsHeaderNameExpr(call.getArgumentForName("name"))
70+
or
71+
isContentSecurityPolicyHeaderNameExpr(call.getArgumentForName("name")) and
72+
containsFrameAncestorsDirectiveExpr(call.getArgumentForName("value"))
73+
)
74+
}
75+
76+
private predicate isClickjackingHeaderDictionaryLikeWrite(Call call) {
77+
(
78+
call.getTarget().hasUndecoratedName(["Append", "Add", "TryAdd"])
79+
or
80+
call.(IndexerCall).getTarget() instanceof Setter
81+
) and
82+
(
83+
isXFrameOptionsHeaderNameExpr(call.getArgumentForName("key"))
84+
or
85+
isContentSecurityPolicyHeaderNameExpr(call.getArgumentForName("key")) and
86+
containsFrameAncestorsDirectiveExpr(call.getArgumentForName("value"))
87+
)
88+
}
89+
90+
private predicate isClickjackingPropertyWrite(Call c) {
91+
c.getTarget() instanceof Setter and
92+
(
93+
c.(PropertyCall).getProperty() =
94+
any(MicrosoftAspNetCoreHttpIHeaderDictionary dic).getXFrameOptionsProperty()
95+
or
96+
c.(PropertyCall).getProperty() =
97+
any(MicrosoftAspNetCoreHttpIHeaderDictionary dic).getContentSecurityPolicyProperty() and
98+
containsFrameAncestorsDirectiveExpr(c.getArgumentForName("value"))
99+
)
100+
}
101+
102+
/** Gets an expression that configures a clickjacking-related response header. */
103+
Call getAClickjackingHeaderWrite() {
104+
isClickjackingHeaderCall(result)
105+
or
106+
isCallOnResponseHeadersAccess(result) and
107+
(
108+
isClickjackingHeaderDictionaryLikeWrite(result)
109+
or
110+
isClickjackingPropertyWrite(result)
111+
)
112+
}

‎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 semmle.code.csharp.security.MissingXFrameOptionsQuery
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+
isXFrameOptionsText(name)
57+
or
58+
isContentSecurityPolicyText(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/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: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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"] = // $ Alert
18+
"default-src 'self'; frame-ancestors 'none'";
19+
context.Response.Headers.XFrameOptions = "DENY"; // $ Alert
20+
context.Response.Headers.ContentSecurityPolicy = // $ Alert
21+
"default-src 'self'; frame-ancestors 'self'";
22+
context.Response.Headers["Content-Security-Policy"] = // $ Alert
23+
"default-src 'self', frame-ancestors 'none'";
24+
25+
IHeaderDictionary responseHeaders = context.Response.Headers;
26+
responseHeaders.Append("X-Frame-Options", "DENY"); // $ Alert
27+
}
28+
29+
public void IgnoredHeaderWrites(AspNetCoreHttpContext context)
30+
{
31+
context.Request.Headers["X-Frame-Options"] = "DENY";
32+
33+
IHeaderDictionary reassignedHeaders = context.Response.Headers;
34+
reassignedHeaders = context.Request.Headers;
35+
reassignedHeaders["X-Frame-Options"] = "DENY";
36+
37+
var standaloneHeaders = new HeaderDictionary();
38+
standaloneHeaders.Append("X-Frame-Options", "DENY");
39+
standaloneHeaders["Content-Security-Policy"] = "frame-ancestors 'none'";
40+
41+
context.Response.Headers["Content-Security-Policy-Report-Only"] =
42+
"frame-ancestors 'none'";
43+
context.Response.Headers.ContentSecurityPolicyReportOnly = "frame-ancestors 'none'";
44+
context.Response.Headers["X-Content-Security-Policy"] = "frame-ancestors 'none'";
45+
context.Response.Headers["Content-Security-Policy"] = "default-src 'self'";
46+
context.Response.Headers["Content-Security-Policy"] =
47+
"report-uri https://example.test/frame-ancestors";
48+
context.Response.Headers["Content-Security-Policy"] =
49+
"default-src 'self'; not-frame-ancestors 'none'";
50+
}
51+
}

‎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:51 | access to indexer | A clickjacking-related response header is configured here. |
5+
| HeaderWrites.cs:17:9:17:59 | access to indexer | A clickjacking-related response header is configured here. |
6+
| HeaderWrites.cs:19:9:19:46 | access to property XFrameOptions | A clickjacking-related response header is configured here. |
7+
| HeaderWrites.cs:20:9:20:54 | access to property ContentSecurityPolicy | A clickjacking-related response header is configured here. |
8+
| HeaderWrites.cs:22:9:22:59 | access to indexer | A clickjacking-related response header is configured here. |
9+
| HeaderWrites.cs:26:9:26:57 | call to method Append | A clickjacking-related response header is configured here. |

0 commit comments

Comments
 (0)