Skip to content

Commit 3d1eb19

Browse files
michaelnebelBubby4j
michaelnebel
authored and
Bubby4j
committed
C#: Some re-factoring.
1 parent 66d4840 commit 3d1eb19

6 files changed

Lines changed: 84 additions & 74 deletions

File tree

‎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: 44 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import semmle.code.csharp.frameworks.system.Web
77

88
/** Holds if `name` is the `X-Frame-Options` header name, ignoring case. */
99
bindingset[name]
10-
predicate isXFrameOptionsHeaderName(string name) { name.toLowerCase() = "x-frame-options" }
10+
predicate isXFrameOptionsText(string name) { name.toLowerCase() = "x-frame-options" }
1111

1212
/** Holds if `name` is the enforced `Content-Security-Policy` header name, ignoring case. */
1313
bindingset[name]
14-
predicate isContentSecurityPolicyHeaderName(string name) {
14+
predicate isContentSecurityPolicyText(string name) {
1515
name.toLowerCase() = "content-security-policy"
1616
}
1717

@@ -24,49 +24,42 @@ predicate containsFrameAncestorsDirective(string value) {
2424
value.regexpMatch("(?is)(^|.*[;,])\\s*frame-ancestors(\\s|;|$).*")
2525
}
2626

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-
3427
private predicate isXFrameOptionsHeaderNameExpr(Expr name) {
35-
isXFrameOptionsHeaderName(name.stripImplicit().getValue())
28+
isXFrameOptionsText(name.stripImplicit().getValue())
3629
or
37-
isHeaderNamesField(name, "XFrameOptions")
30+
name.stripImplicit().(FieldAccess).getTarget() =
31+
any(MicrosoftNetHttpHeadersHeaderNames f).getXFrameOptionsField()
3832
}
3933

4034
private predicate isContentSecurityPolicyHeaderNameExpr(Expr name) {
41-
isContentSecurityPolicyHeaderName(name.stripImplicit().getValue())
42-
or
43-
isHeaderNamesField(name, "ContentSecurityPolicy")
35+
isContentSecurityPolicyText(name.stripImplicit().getValue()) or
36+
name.stripImplicit().(FieldAccess).getTarget() =
37+
any(MicrosoftNetHttpHeadersHeaderNames f).getContentSecurityPolicyField()
4438
}
4539

4640
private predicate containsFrameAncestorsDirectiveExpr(Expr value) {
4741
containsFrameAncestorsDirective(value.stripImplicit().getValue())
4842
}
4943

5044
private predicate isDirectResponseHeadersAccess(Expr expr) {
51-
exists(PropertyAccessExpr headers, MicrosoftAspNetCoreHttpHttpResponse response |
45+
exists(PropertyAccess headers, MicrosoftAspNetCoreHttpHttpResponse response |
5246
expr.stripImplicit() = headers and headers.getProperty() = response.getHeadersProperty()
5347
)
5448
}
5549

56-
private predicate isResponseHeadersAccess(Expr expr) {
57-
exists(Expr directAccess |
58-
isDirectResponseHeadersAccess(directAccess) and
59-
DataFlow::localExprFlow(directAccess, expr.stripImplicit())
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+
)
6060
)
6161
}
6262

63-
private Expr getHeaderDictionaryReceiver(MethodCall call) {
64-
result = call.getQualifier()
65-
or
66-
call.getTarget().isExtensionMethod() and
67-
result = call.getArgumentForParameter(call.getTarget().getParameter(0))
68-
}
69-
7063
private predicate isClickjackingHeaderCall(MethodCall call) {
7164
(
7265
call.getTarget() = any(SystemWebHttpResponseClass r).getAppendHeaderMethod() or
@@ -78,9 +71,14 @@ private predicate isClickjackingHeaderCall(MethodCall call) {
7871
isContentSecurityPolicyHeaderNameExpr(call.getArgumentForName("name")) and
7972
containsFrameAncestorsDirectiveExpr(call.getArgumentForName("value"))
8073
)
81-
or
82-
call.getTarget().hasUndecoratedName(["Append", "Add", "TryAdd"]) and
83-
isResponseHeadersAccess(getHeaderDictionaryReceiver(call)) and
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
8482
(
8583
isXFrameOptionsHeaderNameExpr(call.getArgumentForName("key"))
8684
or
@@ -89,39 +87,26 @@ private predicate isClickjackingHeaderCall(MethodCall call) {
8987
)
9088
}
9189

92-
private predicate isClickjackingHeaderIndexerAssignment(AssignExpr assignment) {
93-
exists(IndexerCall indexer |
94-
assignment.getLeftOperand() = indexer and
95-
isResponseHeadersAccess(indexer.getQualifier()) and
96-
(
97-
isXFrameOptionsHeaderNameExpr(indexer.getArgument(0))
98-
or
99-
isContentSecurityPolicyHeaderNameExpr(indexer.getArgument(0)) and
100-
containsFrameAncestorsDirectiveExpr(assignment.getRightOperand())
101-
)
102-
)
103-
}
104-
105-
private predicate isClickjackingNamedHeaderPropertyAssignment(AssignExpr assignment) {
106-
exists(PropertyAccessExpr header |
107-
assignment.getLeftOperand() = header and
108-
isResponseHeadersAccess(header.(QualifiableExpr).getQualifier()) and
109-
(
110-
header.getProperty().hasName("XFrameOptions")
111-
or
112-
header.getProperty().hasName("ContentSecurityPolicy") and
113-
containsFrameAncestorsDirectiveExpr(assignment.getRightOperand())
114-
)
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"))
11599
)
116100
}
117101

118102
/** Gets an expression that configures a clickjacking-related response header. */
119-
Expr getAClickjackingHeaderWrite() {
120-
result = any(MethodCall call | isClickjackingHeaderCall(call))
103+
Call getAClickjackingHeaderWrite() {
104+
isClickjackingHeaderCall(result)
121105
or
122-
result =
123-
any(AssignExpr assignment |
124-
isClickjackingHeaderIndexerAssignment(assignment) or
125-
isClickjackingNamedHeaderPropertyAssignment(assignment)
126-
)
106+
isCallOnResponseHeadersAccess(result) and
107+
(
108+
isClickjackingHeaderDictionaryLikeWrite(result)
109+
or
110+
isClickjackingPropertyWrite(result)
111+
)
127112
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ predicate hasWebConfigClickjackingProtection(WebConfigXml webConfig) {
5353
.getAChild("add") and
5454
name = add.getAttributeValue("name") and
5555
(
56-
isXFrameOptionsHeaderName(name)
56+
isXFrameOptionsText(name)
5757
or
58-
isContentSecurityPolicyHeaderName(name) and
58+
isContentSecurityPolicyText(name) and
5959
containsFrameAncestorsDirective(add.getAttributeValue("value"))
6060
)
6161
)

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,13 @@ public void AspNetCoreResponseHeaders(AspNetCoreHttpContext context)
1414
"default-src 'self'; FrAmE-AnCeStOrS 'none'"); // $ Alert
1515

1616
context.Response.Headers["X-Frame-Options"] = "DENY"; // $ Alert
17-
context.Response.Headers["Content-Security-Policy"] =
18-
"default-src 'self'; frame-ancestors 'none'"; // $ Alert
19-
17+
context.Response.Headers["Content-Security-Policy"] = // $ Alert
18+
"default-src 'self'; frame-ancestors 'none'";
2019
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
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'";
2524

2625
IHeaderDictionary responseHeaders = context.Response.Headers;
2726
responseHeaders.Append("X-Frame-Options", "DENY"); // $ Alert

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
| HeaderWrites.cs:10:9:10:74 | call to method Append | A clickjacking-related response header is configured here. |
22
| HeaderWrites.cs:11:9:11:69 | call to method Add | A clickjacking-related response header is configured here. |
33
| 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. |
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)