C#: Add query for insecure certificate validation

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

@@ -0,0 +1,29 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+      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.
+    </p>
+  </overview>
+  <recommendation>
+    <p>
+      Ensure that <code>RemoteCertificateValidationCallback</code> implementations properly verify the certificate before returning <code>true</code>. Avoid implementing callbacks that unconditionally accept all certificates.
+    </p>
+  </recommendation>
+  <example>
+    <p>
+      The following example demonstrates an insecure use of <code>RemoteCertificateValidationCallback</code> that always returns <code>true</code>:
+    </p>
+    <sample src="InsecureCertificateValidationExample.cs"/>
+    <p>
+      A secure approach would involve proper verification of the certificate before returning <code>true</code>:
+    </p>
+    <sample src="SecureCertificateValidationExample.cs"/>
+  </example>
+  <references>
+    <li>
+      CA5359: Do not disable certificate validation
+      <a href="https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca5359">CA5359</a>
+    </li>
+  </references>
+</qhelp>

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

@@ -0,0 +1,100 @@
+/**
+ * @name Unsafe `CertificateValidationCallback` use.
+ * @description Using a `CertificateValidationCallback` that always returns `true` is insecure, as it allows any certificate to be accepted as valid.
+ * @kind alert
+ * @problem.severity error
+ * @precision high
+ * @id cs/unsafe-certificate-validation
+ * @tags security
+ *       external/cwe/cwe-295
+ *       external/cwe/cwe-297
+ */
+
+import csharp
+
+/**
+ * Provides an abstract base class for properties related to certificate validation.
+ */
+abstract private class CertificateValidationProperty extends Property { }
+
+/**
+ * Represents the `ServerCertificateValidationCallback` property of the `ServicePointManager` class.
+ */
+private class ServicePointManagerServerCertificateValidationCallback extends CertificateValidationProperty
+{
+  ServicePointManagerServerCertificateValidationCallback() {
+    this.getDeclaringType().hasFullyQualifiedName("System.Net", "ServicePointManager") and
+    this.hasName("ServerCertificateValidationCallback")
+  }
+}
+
+/**
+ * Represents the `ServerCertificateCustomValidationCallback` property of the `HttpClientHandler` class.
+ */
+private class HttpClientHandlerServerCertificateCustomValidationCallback extends CertificateValidationProperty
+{
+  HttpClientHandlerServerCertificateCustomValidationCallback() {
+    this.getDeclaringType().hasFullyQualifiedName("System.Net.Http", "HttpClientHandler") and
+    this.hasName("ServerCertificateCustomValidationCallback")
+  }
+}
+
+/**
+ * Represents the creation of an `SslStream` object.
+ */
+private class SslStreamCreation extends ObjectCreation {
+  SslStreamCreation() {
+    this.getObjectType().hasFullyQualifiedName("System.Net.Security", "SslStream")
+  }
+
+  /**
+   * Gets the expression used as the server certificate validation callback argument
+   * in the creation of the `SslStream` object.
+   */
+  Expr getServerCertificateValidationCallback() { result = this.getArgument(2) }
+}
+
+/**
+ * Represents the `ServerCertificateValidationCallback` property of the `HttpWebRequest` class.
+ */
+private class HttpWebRequestServerCertificateValidationCallback extends CertificateValidationProperty
+{
+  HttpWebRequestServerCertificateValidationCallback() {
+    this.getDeclaringType().hasFullyQualifiedName("System.Net", "HttpWebRequest") and
+    this.hasName("ServerCertificateValidationCallback")
+  }
+}
+
+/**
+ * Holds if `c` always returns `true`.
+ */
+private predicate alwaysReturnsTrue(Callable c) {
+  forex(ReturnStmt rs | rs.getEnclosingCallable() = c |
+    rs.getExpr().(BoolLiteral).getBoolValue() = true
+  )
+  or
+  c.getBody().(BoolLiteral).getBoolValue() = true
+}
+
+/**
+ * Gets the actual callable object referred to by expression `e`.
+ * This can be a direct reference to a callable, a delegate creation, or an anonymous function.
+ */
+Callable getActualCallable(Expr e) {
+  exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() |
+    result = dcArg.(CallableAccess).getTarget() or result = dcArg.(AnonymousFunctionExpr)
+  )
+  or
+  result = e.(Callable)
+}
+
+from Expr e, Callable c
+where
+  [
+    any(SslStreamCreation yy).getServerCertificateValidationCallback(),
+    any(CertificateValidationProperty xx).getAnAssignedValue()
+  ] = e and
+  getActualCallable(e) = c and
+  alwaysReturnsTrue(c)
+select e, "$@ that is defined $@ and accepts any certificate as valid, is used here.", e,
+  "This certificate callback", c, "here"

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

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

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

@@ -0,0 +1,6 @@
+ServicePointManager.ServerCertificateValidationCallback += 
+        (sender, cert, chain, sslPolicyErrors) => {
+            if (cert.Issuer == "TrustedIssuer" /* && other conditions */)
+                return true;
+            return false;
+        };

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

@@ -0,0 +1,6 @@
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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 |
+| 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

@@ -0,0 +1 @@
+experimental/CWE-295/InsecureCertificateValidation.ql

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

@@ -0,0 +1,80 @@
+// 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
+using System;
+using System.Net;
+using System.Net.Security;
+using System.Net.Sockets;
+using System.Security.Cryptography.X509Certificates;
+using System.IO;
+using System.Net.Http;
+
+class Program
+{
+
+    static void First()
+    {
+        TcpClient client = new TcpClient("www.example.com", 443);
+        SslStream sslStream = new SslStream(
+            client.GetStream(),
+            false,
+            new RemoteCertificateValidationCallback(ValidateServerCertificate), // BAD: unsafe callback used
+            null
+        );
+        sslStream = new SslStream(
+           client.GetStream(),
+           false,
+           new RemoteCertificateValidationCallback(SafeValidateServerCertificate), // GOOD: safe callback used
+           null
+       );
+
+        try
+        {
+            sslStream.AuthenticateAsClient("www.example.com");
+        }
+        catch (Exception e)
+        {
+            Console.WriteLine(e.Message);
+        }
+    }
+
+    public static bool ValidateServerCertificate(
+          object sender,
+          X509Certificate certificate,
+          X509Chain chain,
+          SslPolicyErrors sslPolicyErrors)
+    {
+        return true;
+    }
+
+    public static bool SafeValidateServerCertificate(
+      object sender,
+      X509Certificate certificate,
+      X509Chain chain,
+      SslPolicyErrors sslPolicyErrors)
+    {
+        return sslPolicyErrors == SslPolicyErrors.None;
+    }
+
+    static void Second()
+    {
+        HttpClientHandler handler = new HttpClientHandler();
+        handler.ServerCertificateCustomValidationCallback = (sender, cert, chain, sslPolicyErrors) => true; // BAD: unsafe callback used
+        handler.ServerCertificateCustomValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
+        HttpClient client = new HttpClient(handler);
+    }
+
+    static void Third()
+    {
+        ServicePointManager.ServerCertificateValidationCallback = new RemoteCertificateValidationCallback(ValidateServerCertificate); // BAD: unsafe callback used
+        ServicePointManager.ServerCertificateValidationCallback = (sender, cert, chain, sslPolicyErrors) => true; // BAD: unsafe callback used
+        ServicePointManager.ServerCertificateValidationCallback = ValidateServerCertificate; // BAD: unsafe callback used
+        ServicePointManager.ServerCertificateValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
+    }
+    static void Fourth()
+    {
+        HttpWebRequest request = (HttpWebRequest)WebRequest.Create("https://www.example.com");
+        request.ServerCertificateValidationCallback = ValidateServerCertificate; // BAD: unsafe callback used
+        request.ServerCertificateValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
+
+    }
+
+}