Skip to content

Commit e67e8a9

Browse files
intrigus-lgtmmichaelnebel
intrigus-lgtm
authored and
michaelnebel
committed
C#: Add query for insecure certificate validation
1 parent 76914c4 commit e67e8a9

7 files changed

+224
-0
lines changed

‎csharp/ql/src/experimental/CWE-295/InsecureCertificateValidation.qhelp

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
Using a <code>RemoteCertificateValidationCallback</code> that always returns <code>true</code> is insecure because it allows any certificate to be accepted as valid. This can lead to a variety of security issues, including man-in-the-middle attacks.
6+
</p>
7+
</overview>
8+
<recommendation>
9+
<p>
10+
Ensure that <code>RemoteCertificateValidationCallback</code> implementations properly verify the certificate before returning <code>true</code>. Avoid implementing callbacks that unconditionally accept all certificates.
11+
</p>
12+
</recommendation>
13+
<example>
14+
<p>
15+
The following example demonstrates an insecure use of <code>RemoteCertificateValidationCallback</code> that always returns <code>true</code>:
16+
</p>
17+
<sample src="InsecureCertificateValidationExample.cs"/>
18+
<p>
19+
A secure approach would involve proper verification of the certificate before returning <code>true</code>:
20+
</p>
21+
<sample src="SecureCertificateValidationExample.cs"/>
22+
</example>
23+
<references>
24+
<li>
25+
CA5359: Do not disable certificate validation
26+
<a href="https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca5359">CA5359</a>
27+
</li>
28+
</references>
29+
</qhelp>

‎csharp/ql/src/experimental/CWE-295/InsecureCertificateValidation.ql

+100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/**
2+
* @name Unsafe `CertificateValidationCallback` use.
3+
* @description Using a `CertificateValidationCallback` that always returns `true` is insecure, as it allows any certificate to be accepted as valid.
4+
* @kind alert
5+
* @problem.severity error
6+
* @precision high
7+
* @id cs/unsafe-certificate-validation
8+
* @tags security
9+
* external/cwe/cwe-295
10+
* external/cwe/cwe-297
11+
*/
12+
13+
import csharp
14+
15+
/**
16+
* Provides an abstract base class for properties related to certificate validation.
17+
*/
18+
abstract private class CertificateValidationProperty extends Property { }
19+
20+
/**
21+
* Represents the `ServerCertificateValidationCallback` property of the `ServicePointManager` class.
22+
*/
23+
private class ServicePointManagerServerCertificateValidationCallback extends CertificateValidationProperty
24+
{
25+
ServicePointManagerServerCertificateValidationCallback() {
26+
this.getDeclaringType().hasFullyQualifiedName("System.Net", "ServicePointManager") and
27+
this.hasName("ServerCertificateValidationCallback")
28+
}
29+
}
30+
31+
/**
32+
* Represents the `ServerCertificateCustomValidationCallback` property of the `HttpClientHandler` class.
33+
*/
34+
private class HttpClientHandlerServerCertificateCustomValidationCallback extends CertificateValidationProperty
35+
{
36+
HttpClientHandlerServerCertificateCustomValidationCallback() {
37+
this.getDeclaringType().hasFullyQualifiedName("System.Net.Http", "HttpClientHandler") and
38+
this.hasName("ServerCertificateCustomValidationCallback")
39+
}
40+
}
41+
42+
/**
43+
* Represents the creation of an `SslStream` object.
44+
*/
45+
private class SslStreamCreation extends ObjectCreation {
46+
SslStreamCreation() {
47+
this.getObjectType().hasFullyQualifiedName("System.Net.Security", "SslStream")
48+
}
49+
50+
/**
51+
* Gets the expression used as the server certificate validation callback argument
52+
* in the creation of the `SslStream` object.
53+
*/
54+
Expr getServerCertificateValidationCallback() { result = this.getArgument(2) }
55+
}
56+
57+
/**
58+
* Represents the `ServerCertificateValidationCallback` property of the `HttpWebRequest` class.
59+
*/
60+
private class HttpWebRequestServerCertificateValidationCallback extends CertificateValidationProperty
61+
{
62+
HttpWebRequestServerCertificateValidationCallback() {
63+
this.getDeclaringType().hasFullyQualifiedName("System.Net", "HttpWebRequest") and
64+
this.hasName("ServerCertificateValidationCallback")
65+
}
66+
}
67+
68+
/**
69+
* Holds if `c` always returns `true`.
70+
*/
71+
private predicate alwaysReturnsTrue(Callable c) {
72+
forex(ReturnStmt rs | rs.getEnclosingCallable() = c |
73+
rs.getExpr().(BoolLiteral).getBoolValue() = true
74+
)
75+
or
76+
c.getBody().(BoolLiteral).getBoolValue() = true
77+
}
78+
79+
/**
80+
* Gets the actual callable object referred to by expression `e`.
81+
* This can be a direct reference to a callable, a delegate creation, or an anonymous function.
82+
*/
83+
Callable getActualCallable(Expr e) {
84+
exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() |
85+
result = dcArg.(CallableAccess).getTarget() or result = dcArg.(AnonymousFunctionExpr)
86+
)
87+
or
88+
result = e.(Callable)
89+
}
90+
91+
from Expr e, Callable c
92+
where
93+
[
94+
any(SslStreamCreation yy).getServerCertificateValidationCallback(),
95+
any(CertificateValidationProperty xx).getAnAssignedValue()
96+
] = e and
97+
getActualCallable(e) = c and
98+
alwaysReturnsTrue(c)
99+
select e, "$@ that is defined $@ and accepts any certificate as valid, is used here.", e,
100+
"This certificate callback", c, "here"

‎csharp/ql/src/experimental/CWE-295/InsecureCertificateValidationExample.cs

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ServicePointManager.ServerCertificateValidationCallback =
2+
(sender, cert, chain, sslPolicyErrors) => true;

‎csharp/ql/src/experimental/CWE-295/SecureCertificateValidationExample.cs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
ServicePointManager.ServerCertificateValidationCallback +=
2+
(sender, cert, chain, sslPolicyErrors) => {
3+
if (cert.Issuer == "TrustedIssuer" /* && other conditions */)
4+
return true;
5+
return false;
6+
};

‎csharp/ql/test/experimental/CWE-295/InsecureCertificateValidation.expected

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
| Program.cs:19:13:19:78 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:19:13:19:78 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:39:24:39:48 | ValidateServerCertificate | here |
2+
| Program.cs:60:61:60:106 | (...) => ... | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:60:61:60:106 | (...) => ... | This certificate callback | Program.cs:60:61:60:106 | (...) => ... | here |
3+
| Program.cs:67:67:67:132 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:67:67:67:132 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:39:24:39:48 | ValidateServerCertificate | here |
4+
| Program.cs:68:67:68:112 | (...) => ... | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:68:67:68:112 | (...) => ... | This certificate callback | Program.cs:68:67:68:112 | (...) => ... | here |
5+
| Program.cs:69:67:69:91 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:69:67:69:91 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:39:24:39:48 | ValidateServerCertificate | here |
6+
| Program.cs:75:55:75:79 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:75:55:75:79 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:39:24:39:48 | ValidateServerCertificate | here |

‎csharp/ql/test/experimental/CWE-295/InsecureCertificateValidation.qlref

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/CWE-295/InsecureCertificateValidation.ql

‎csharp/ql/test/experimental/CWE-295/Program.cs

+80
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// semmle-extractor-options: /r:System.Net.Sockets.dll /r:System.Net.Security.dll /r:System.Security.Cryptography.Algorithms.dll /r:System.Net.Http.dll /r:System.Net.ServicePoint.dll /r:System.Security.Cryptography.dll /r:System.Net.Primitives.dll /r:System.Net.Requests.dll /r:System.Private.Uri.dll
2+
using System;
3+
using System.Net;
4+
using System.Net.Security;
5+
using System.Net.Sockets;
6+
using System.Security.Cryptography.X509Certificates;
7+
using System.IO;
8+
using System.Net.Http;
9+
10+
class Program
11+
{
12+
13+
static void First()
14+
{
15+
TcpClient client = new TcpClient("www.example.com", 443);
16+
SslStream sslStream = new SslStream(
17+
client.GetStream(),
18+
false,
19+
new RemoteCertificateValidationCallback(ValidateServerCertificate), // BAD: unsafe callback used
20+
null
21+
);
22+
sslStream = new SslStream(
23+
client.GetStream(),
24+
false,
25+
new RemoteCertificateValidationCallback(SafeValidateServerCertificate), // GOOD: safe callback used
26+
null
27+
);
28+
29+
try
30+
{
31+
sslStream.AuthenticateAsClient("www.example.com");
32+
}
33+
catch (Exception e)
34+
{
35+
Console.WriteLine(e.Message);
36+
}
37+
}
38+
39+
public static bool ValidateServerCertificate(
40+
object sender,
41+
X509Certificate certificate,
42+
X509Chain chain,
43+
SslPolicyErrors sslPolicyErrors)
44+
{
45+
return true;
46+
}
47+
48+
public static bool SafeValidateServerCertificate(
49+
object sender,
50+
X509Certificate certificate,
51+
X509Chain chain,
52+
SslPolicyErrors sslPolicyErrors)
53+
{
54+
return sslPolicyErrors == SslPolicyErrors.None;
55+
}
56+
57+
static void Second()
58+
{
59+
HttpClientHandler handler = new HttpClientHandler();
60+
handler.ServerCertificateCustomValidationCallback = (sender, cert, chain, sslPolicyErrors) => true; // BAD: unsafe callback used
61+
handler.ServerCertificateCustomValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
62+
HttpClient client = new HttpClient(handler);
63+
}
64+
65+
static void Third()
66+
{
67+
ServicePointManager.ServerCertificateValidationCallback = new RemoteCertificateValidationCallback(ValidateServerCertificate); // BAD: unsafe callback used
68+
ServicePointManager.ServerCertificateValidationCallback = (sender, cert, chain, sslPolicyErrors) => true; // BAD: unsafe callback used
69+
ServicePointManager.ServerCertificateValidationCallback = ValidateServerCertificate; // BAD: unsafe callback used
70+
ServicePointManager.ServerCertificateValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
71+
}
72+
static void Fourth()
73+
{
74+
HttpWebRequest request = (HttpWebRequest)WebRequest.Create("https://www.example.com");
75+
request.ServerCertificateValidationCallback = ValidateServerCertificate; // BAD: unsafe callback used
76+
request.ServerCertificateValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
77+
78+
}
79+
80+
}

0 commit comments

Comments
 (0)