Skip to content

Commit 677afff

Browse files
CopilotCopilot
authored
Add CWE-295 C# query for accepting any TLS certificate
1 parent f591987 commit 677afff

8 files changed

Lines changed: 295 additions & 0 deletions

File tree

‎csharp/ql/src/Security Features/CWE-295/AcceptAnyCertificate.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using System.Net.Http;
2+
using System.Net.Security;
3+
using System.Security.Cryptography.X509Certificates;
4+
5+
public class CertificateValidation
6+
{
7+
public void Bad()
8+
{
9+
var handler = new HttpClientHandler();
10+
// BAD: the callback always returns true, so every certificate is trusted.
11+
handler.ServerCertificateCustomValidationCallback =
12+
(request, certificate, chain, errors) => true;
13+
}
14+
15+
public void Good()
16+
{
17+
var handler = new HttpClientHandler();
18+
// GOOD: the certificate is only trusted when there are no validation errors.
19+
handler.ServerCertificateCustomValidationCallback =
20+
(request, certificate, chain, errors) => errors == SslPolicyErrors.None;
21+
}
22+
}

‎csharp/ql/src/Security Features/CWE-295/AcceptAnyCertificate.qhelp‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
A TLS/SSL certificate validation callback that always returns <code>true</code> trusts every certificate,
8+
regardless of any validation errors that were detected. This allows an attacker to perform a machine-in-the-middle
9+
attack against the application, therefore breaking any security that Transport Layer Security (TLS) provides.
10+
</p>
11+
12+
<p>
13+
An attack might look like this:
14+
</p>
15+
16+
<ol>
17+
<li>The vulnerable program connects to <code>https://example.com</code>.</li>
18+
<li>The attacker intercepts this connection and presents a valid, self-signed certificate for <code>https://example.com</code>.</li>
19+
<li>The vulnerable program calls the certificate validation callback to check whether it should trust the certificate.</li>
20+
<li>The callback ignores the <code>SslPolicyErrors</code> argument and returns <code>true</code>.</li>
21+
<li>The vulnerable program accepts the certificate and proceeds with the connection, since the callback indicated that the certificate is trusted.</li>
22+
<li>The attacker can now read the data the program sends to <code>https://example.com</code> and/or alter its replies while the program thinks the connection is secure.</li>
23+
</ol>
24+
</overview>
25+
26+
<recommendation>
27+
<p>
28+
Do not use a certificate validation callback that unconditionally returns <code>true</code>.
29+
Either rely on the default certificate validation, or implement a callback that inspects the
30+
<code>SslPolicyErrors</code> argument and only trusts a specific, known certificate (for example, when
31+
using a self-signed certificate that has been explicitly pinned).
32+
</p>
33+
</recommendation>
34+
35+
<example>
36+
<p>
37+
In the first (bad) example, the callback always returns <code>true</code> and therefore trusts any certificate,
38+
which allows an attacker to perform a machine-in-the-middle attack. In the second (good) example, the callback
39+
returns <code>true</code> only when there are no validation errors.
40+
</p>
41+
<sample src="AcceptAnyCertificate.cs" />
42+
</example>
43+
44+
<references>
45+
<li>Microsoft Learn:
46+
<a href="https://learn.microsoft.com/en-us/dotnet/api/system.net.security.remotecertificatevalidationcallback">RemoteCertificateValidationCallback Delegate</a>.</li>
47+
<li>Microsoft Learn:
48+
<a href="https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca5359">CA5359: Do not disable certificate validation</a>.</li>
49+
<li>OWASP:
50+
<a href="https://owasp.org/www-community/attacks/Manipulator-in-the-middle_attack">Manipulator-in-the-middle attack</a>.</li>
51+
</references>
52+
</qhelp>

‎csharp/ql/src/Security Features/CWE-295/AcceptAnyCertificate.ql‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/**
2+
* @name Accepting any TLS certificate during validation
3+
* @description A certificate validation callback that always accepts any certificate
4+
* allows an attacker to perform a machine-in-the-middle attack.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id cs/accept-any-certificate
10+
* @tags security
11+
* external/cwe/cwe-295
12+
*/
13+
14+
import csharp
15+
import semmle.code.csharp.dataflow.DataFlow::DataFlow
16+
import AcceptAnyCertificate::PathGraph
17+
18+
/**
19+
* Holds if `c` always returns `true` and never returns `false`, i.e. it accepts
20+
* every input it is given.
21+
*/
22+
predicate alwaysReturnsTrue(Callable c) {
23+
c.getReturnType() instanceof BoolType and
24+
// There is at least one returned value, and every returned value is the
25+
// constant `true`.
26+
forex(Expr ret | c.canReturn(ret) | ret.getValue() = "true")
27+
}
28+
29+
/**
30+
* A delegate type used as a TLS/SSL certificate validation callback. Such a
31+
* delegate returns a `bool` (whether the certificate is trusted) and takes a
32+
* `System.Net.Security.SslPolicyErrors` parameter describing any validation
33+
* errors that were found. This covers `RemoteCertificateValidationCallback` as
34+
* well as the `Func<..., SslPolicyErrors, bool>` callbacks used by, for example,
35+
* `HttpClientHandler.ServerCertificateCustomValidationCallback`.
36+
*/
37+
class CertificateValidationCallbackType extends DelegateType {
38+
CertificateValidationCallbackType() {
39+
this.getReturnType() instanceof BoolType and
40+
this.getAParameter().getType().hasFullyQualifiedName("System.Net.Security", "SslPolicyErrors")
41+
}
42+
}
43+
44+
/**
45+
* Gets a callable that always accepts any certificate, referenced by the
46+
* delegate-producing expression `e`.
47+
*/
48+
Callable getAcceptingCallable(Expr e) {
49+
// A lambda or anonymous method, e.g. `(sender, cert, chain, errors) => true`.
50+
result = e and
51+
alwaysReturnsTrue(e)
52+
or
53+
// A method group, e.g. `AcceptAllCertificates`, possibly wrapped in an
54+
// (implicit or explicit) delegate creation.
55+
result = e.(DelegateCreation).getArgument().(CallableAccess).getTarget() and
56+
alwaysReturnsTrue(result)
57+
or
58+
result = e.(CallableAccess).getTarget() and
59+
alwaysReturnsTrue(result)
60+
}
61+
62+
module AcceptAnyCertificateConfig implements DataFlow::ConfigSig {
63+
predicate isSource(DataFlow::Node source) {
64+
exists(getAcceptingCallable(source.asExpr()))
65+
or
66+
// `HttpClientHandler.DangerousAcceptAnyServerCertificateValidator` is a
67+
// built-in callback that accepts every certificate.
68+
source
69+
.asExpr()
70+
.(PropertyAccess)
71+
.getTarget()
72+
.hasName("DangerousAcceptAnyServerCertificateValidator")
73+
}
74+
75+
predicate isSink(DataFlow::Node sink) {
76+
// The value assigned to a property, field or local of certificate
77+
// validation callback type.
78+
exists(Assignable a |
79+
a.getType() instanceof CertificateValidationCallbackType and
80+
sink.asExpr() = a.getAnAssignedValue()
81+
)
82+
or
83+
// The value passed as a certificate validation callback argument, e.g. to
84+
// the `SslStream` constructor.
85+
exists(Call call, Parameter p |
86+
p = call.getTarget().getAParameter() and
87+
p.getType() instanceof CertificateValidationCallbackType and
88+
sink.asExpr() = call.getArgumentForParameter(p)
89+
)
90+
}
91+
92+
predicate observeDiffInformedIncrementalMode() { any() }
93+
}
94+
95+
module AcceptAnyCertificate = DataFlow::Global<AcceptAnyCertificateConfig>;
96+
97+
from AcceptAnyCertificate::PathNode source, AcceptAnyCertificate::PathNode sink
98+
where AcceptAnyCertificate::flowPath(source, sink)
99+
select sink.getNode(), source, sink,
100+
"This TLS certificate validation $@, which trusts any certificate.", source.getNode(),
101+
"uses a callback"

‎csharp/ql/src/change-notes/2026-06-10-accept-any-certificate.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `cs/accept-any-certificate`, to detect TLS/SSL certificate validation callbacks that always accept any certificate (CWE-295).

‎csharp/ql/test/query-tests/Security Features/CWE-295/AcceptAnyCertificate/AcceptAnyCertificate.expected‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
edges
2+
| Test.cs:64:45:64:52 | access to local variable callback : (...) => ... | Test.cs:67:48:67:55 | access to local variable callback | provenance | |
3+
| Test.cs:65:13:65:56 | (...) => ... : (...) => ... | Test.cs:64:45:64:52 | access to local variable callback : (...) => ... | provenance | |
4+
nodes
5+
| Test.cs:14:13:14:57 | (...) => ... | semmle.label | (...) => ... |
6+
| Test.cs:22:13:25:13 | (...) => ... | semmle.label | (...) => ... |
7+
| Test.cs:33:13:33:74 | access to property DangerousAcceptAnyServerCertificateValidator | semmle.label | access to property DangerousAcceptAnyServerCertificateValidator |
8+
| Test.cs:40:13:40:56 | (...) => ... | semmle.label | (...) => ... |
9+
| Test.cs:52:67:52:75 | delegate creation of type RemoteCertificateValidationCallback | semmle.label | delegate creation of type RemoteCertificateValidationCallback |
10+
| Test.cs:59:13:59:56 | (...) => ... | semmle.label | (...) => ... |
11+
| Test.cs:64:45:64:52 | access to local variable callback : (...) => ... | semmle.label | access to local variable callback : (...) => ... |
12+
| Test.cs:65:13:65:56 | (...) => ... | semmle.label | (...) => ... |
13+
| Test.cs:65:13:65:56 | (...) => ... : (...) => ... | semmle.label | (...) => ... : (...) => ... |
14+
| Test.cs:67:48:67:55 | access to local variable callback | semmle.label | access to local variable callback |
15+
subpaths
16+
#select
17+
| Test.cs:14:13:14:57 | (...) => ... | Test.cs:14:13:14:57 | (...) => ... | Test.cs:14:13:14:57 | (...) => ... | This TLS certificate validation $@, which trusts any certificate. | Test.cs:14:13:14:57 | (...) => ... | uses a callback |
18+
| Test.cs:22:13:25:13 | (...) => ... | Test.cs:22:13:25:13 | (...) => ... | Test.cs:22:13:25:13 | (...) => ... | This TLS certificate validation $@, which trusts any certificate. | Test.cs:22:13:25:13 | (...) => ... | uses a callback |
19+
| Test.cs:33:13:33:74 | access to property DangerousAcceptAnyServerCertificateValidator | Test.cs:33:13:33:74 | access to property DangerousAcceptAnyServerCertificateValidator | Test.cs:33:13:33:74 | access to property DangerousAcceptAnyServerCertificateValidator | This TLS certificate validation $@, which trusts any certificate. | Test.cs:33:13:33:74 | access to property DangerousAcceptAnyServerCertificateValidator | uses a callback |
20+
| Test.cs:40:13:40:56 | (...) => ... | Test.cs:40:13:40:56 | (...) => ... | Test.cs:40:13:40:56 | (...) => ... | This TLS certificate validation $@, which trusts any certificate. | Test.cs:40:13:40:56 | (...) => ... | uses a callback |
21+
| Test.cs:52:67:52:75 | delegate creation of type RemoteCertificateValidationCallback | Test.cs:52:67:52:75 | delegate creation of type RemoteCertificateValidationCallback | Test.cs:52:67:52:75 | delegate creation of type RemoteCertificateValidationCallback | This TLS certificate validation $@, which trusts any certificate. | Test.cs:52:67:52:75 | delegate creation of type RemoteCertificateValidationCallback | uses a callback |
22+
| Test.cs:59:13:59:56 | (...) => ... | Test.cs:59:13:59:56 | (...) => ... | Test.cs:59:13:59:56 | (...) => ... | This TLS certificate validation $@, which trusts any certificate. | Test.cs:59:13:59:56 | (...) => ... | uses a callback |
23+
| Test.cs:65:13:65:56 | (...) => ... | Test.cs:65:13:65:56 | (...) => ... | Test.cs:65:13:65:56 | (...) => ... | This TLS certificate validation $@, which trusts any certificate. | Test.cs:65:13:65:56 | (...) => ... | uses a callback |
24+
| Test.cs:67:48:67:55 | access to local variable callback | Test.cs:65:13:65:56 | (...) => ... : (...) => ... | Test.cs:67:48:67:55 | access to local variable callback | This TLS certificate validation $@, which trusts any certificate. | Test.cs:65:13:65:56 | (...) => ... | uses a callback |

‎csharp/ql/test/query-tests/Security Features/CWE-295/AcceptAnyCertificate/AcceptAnyCertificate.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security Features/CWE-295/AcceptAnyCertificate.ql

‎csharp/ql/test/query-tests/Security Features/CWE-295/AcceptAnyCertificate/Test.cs‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
using System.IO;
2+
using System.Net;
3+
using System.Net.Http;
4+
using System.Net.Security;
5+
using System.Security.Cryptography.X509Certificates;
6+
7+
public class CertificateValidationTests
8+
{
9+
public void HttpClientHandlerBad()
10+
{
11+
var handler = new HttpClientHandler();
12+
// BAD: always trusts any certificate.
13+
handler.ServerCertificateCustomValidationCallback =
14+
(request, certificate, chain, errors) => true;
15+
}
16+
17+
public void HttpClientHandlerBlockBodyBad()
18+
{
19+
var handler = new HttpClientHandler();
20+
// BAD: always trusts any certificate.
21+
handler.ServerCertificateCustomValidationCallback =
22+
(request, certificate, chain, errors) =>
23+
{
24+
return true;
25+
};
26+
}
27+
28+
public void HttpClientHandlerDangerousBad()
29+
{
30+
var handler = new HttpClientHandler();
31+
// BAD: built-in callback that accepts any certificate.
32+
handler.ServerCertificateCustomValidationCallback =
33+
HttpClientHandler.DangerousAcceptAnyServerCertificateValidator;
34+
}
35+
36+
public void ServicePointManagerBad()
37+
{
38+
// BAD: always trusts any certificate.
39+
ServicePointManager.ServerCertificateValidationCallback =
40+
(sender, certificate, chain, errors) => true;
41+
}
42+
43+
private static bool AcceptAll(object sender, X509Certificate certificate, X509Chain chain,
44+
SslPolicyErrors errors)
45+
{
46+
return true;
47+
}
48+
49+
public void MethodGroupBad()
50+
{
51+
// BAD: the referenced method always returns true.
52+
ServicePointManager.ServerCertificateValidationCallback = AcceptAll;
53+
}
54+
55+
public void SslStreamBad(Stream stream)
56+
{
57+
// BAD: the validation callback always returns true.
58+
var ssl = new SslStream(stream, false,
59+
(sender, certificate, chain, errors) => true);
60+
}
61+
62+
public void IndirectBad(Stream stream)
63+
{
64+
RemoteCertificateValidationCallback callback =
65+
(sender, certificate, chain, errors) => true;
66+
// BAD: the callback flowing here always returns true.
67+
var ssl = new SslStream(stream, false, callback);
68+
}
69+
70+
public void HttpClientHandlerGood()
71+
{
72+
var handler = new HttpClientHandler();
73+
// GOOD: the certificate is only trusted when there are no validation errors.
74+
handler.ServerCertificateCustomValidationCallback =
75+
(request, certificate, chain, errors) => errors == SslPolicyErrors.None;
76+
}
77+
78+
private static bool Validate(object sender, X509Certificate certificate, X509Chain chain,
79+
SslPolicyErrors errors)
80+
{
81+
return errors == SslPolicyErrors.None;
82+
}
83+
84+
public void MethodGroupGood()
85+
{
86+
// GOOD: the referenced method performs real validation.
87+
ServicePointManager.ServerCertificateValidationCallback = Validate;
88+
}
89+
}

‎csharp/ql/test/query-tests/Security Features/CWE-295/AcceptAnyCertificate/options‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
semmle-extractor-options: /nostdlib /noconfig
2+
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj

0 commit comments

Comments
 (0)