C#: 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,22 @@
+/**
+ * @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 path-problem
+ * @problem.severity error
+ * @precision high
+ * @id cs/unsafe-certificate-validation
+ * @tags security
+ *       external/cwe/cwe-295
+ *       external/cwe/cwe-297
+ */
+
+import csharp
+import DataFlow
+import InsecureCertificateValidationQuery
+import InsecureCertificateValidation::PathGraph
+
+from InsecureCertificateValidation::PathNode source, InsecureCertificateValidation::PathNode sink
+where InsecureCertificateValidation::flowPath(source, sink)
+select sink.getNode(), source, sink,
+  "$@ that is defined $@ and accepts any certificate as valid, is used here.", sink,
+  "This certificate callback", source, "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/InsecureCertificateValidationQuery.qll

@@ -0,0 +1,152 @@
+/**
+ * Provides a taint-tracking configuration for reasoning
+ * about insecure certificate validation vulnerabilities.
+ */
+
+private import csharp
+private import DataFlow
+private import semmle.code.csharp.controlflow.Guards
+
+/**
+ * A source specific to unsafe certificate validation vulnerabilities.
+ */
+abstract private class Source extends DataFlow::Node { }
+
+/**
+ * A sink specific to unsafe certificate validation vulnerabilities.
+ */
+abstract private class Sink extends DataFlow::Node { }
+
+/**
+ * A sanitizer specific to unsafe certificate validation vulnerabilities.
+ */
+abstract private class Sanitizer extends DataFlow::ExprNode { }
+
+/**
+ * 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 `ServerCertificateValidationCallback` property of the `HttpWebRequest` class.
+ */
+private class HttpWebRequestServerCertificateValidationCallback extends CertificateValidationProperty
+{
+  HttpWebRequestServerCertificateValidationCallback() {
+    this.getDeclaringType().hasFullyQualifiedName("System.Net", "HttpWebRequest") and
+    this.hasName("ServerCertificateValidationCallback")
+  }
+}
+
+/**
+ * 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) }
+}
+
+/**
+ * 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.
+ */
+private Callable getActualCallable(Expr e) {
+  exists(Expr dcArg | dcArg = e.(DelegateCreation).getArgument() |
+    result = dcArg.(CallableAccess).getTarget() or
+    result = dcArg.(AnonymousFunctionExpr)
+  )
+  or
+  result = e
+}
+
+private predicate ignoreCertificateValidation(Guard guard, AbstractValue v) {
+  guard =
+    any(PropertyAccess access |
+      access.getProperty().hasFullyQualifiedName("", "Settings", "IgnoreCertificateValidation") and
+      v.(AbstractValues::BooleanValue).getValue() = true
+    )
+}
+
+private class AddIgnoreCheck extends Sanitizer {
+  AddIgnoreCheck() {
+    exists(Guard g, AbstractValues::BooleanValue v | ignoreCertificateValidation(g, v) |
+      g.controlsNode(this.getControlFlowNode(), v)
+    )
+  }
+}
+
+private class CallableAlwaysReturnsTrue extends Callable {
+  CallableAlwaysReturnsTrue() { alwaysReturnsTrue(this) }
+}
+
+private class AddCallableAlwaysReturnsTrue extends Source {
+  AddCallableAlwaysReturnsTrue() {
+    getActualCallable(this.asExpr()) instanceof CallableAlwaysReturnsTrue
+  }
+}
+
+private class AddSinks extends Sink {
+  AddSinks() {
+    exists(Expr e | e = this.asExpr() |
+      e =
+        [
+          any(CertificateValidationProperty p).getAnAssignedValue(),
+          any(SslStreamCreation yy).getServerCertificateValidationCallback()
+        ] and
+      not e.getFile().getAbsolutePath().matches("example")
+    )
+  }
+}
+
+private module InsecureCertificateValidationConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof Source }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+
+  predicate isSink(DataFlow::Node node) { node instanceof Sink }
+}
+
+/**
+ * A taint-tracking module for insecure certificate validation vulnerabilities.
+ */
+module InsecureCertificateValidation = TaintTracking::Global<InsecureCertificateValidationConfig>;

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,28 @@
+edges
+| Program.cs:80:58:80:65 | callback : ValidateServerCertificate | Program.cs:82:67:82:74 | access to parameter callback | provenance |  |
+| Program.cs:87:45:87:52 | access to local variable callback : ValidateServerCertificate | Program.cs:89:25:89:32 | access to local variable callback : ValidateServerCertificate | provenance |  |
+| Program.cs:87:56:87:80 | delegate creation of type RemoteCertificateValidationCallback : ValidateServerCertificate | Program.cs:87:45:87:52 | access to local variable callback : ValidateServerCertificate | provenance |  |
+| Program.cs:89:25:89:32 | access to local variable callback : ValidateServerCertificate | Program.cs:80:58:80:65 | callback : ValidateServerCertificate | provenance |  |
+nodes
+| Program.cs:18:13:18:78 | delegate creation of type RemoteCertificateValidationCallback | semmle.label | delegate creation of type RemoteCertificateValidationCallback |
+| Program.cs:59:61:59:106 | (...) => ... | semmle.label | (...) => ... |
+| Program.cs:66:67:66:132 | delegate creation of type RemoteCertificateValidationCallback | semmle.label | delegate creation of type RemoteCertificateValidationCallback |
+| Program.cs:67:67:67:112 | (...) => ... | semmle.label | (...) => ... |
+| Program.cs:68:67:68:91 | delegate creation of type RemoteCertificateValidationCallback | semmle.label | delegate creation of type RemoteCertificateValidationCallback |
+| Program.cs:75:55:75:79 | delegate creation of type RemoteCertificateValidationCallback | semmle.label | delegate creation of type RemoteCertificateValidationCallback |
+| Program.cs:80:58:80:65 | callback : ValidateServerCertificate | semmle.label | callback : ValidateServerCertificate |
+| Program.cs:82:67:82:74 | access to parameter callback | semmle.label | access to parameter callback |
+| Program.cs:87:45:87:52 | access to local variable callback : ValidateServerCertificate | semmle.label | access to local variable callback : ValidateServerCertificate |
+| Program.cs:87:56:87:80 | delegate creation of type RemoteCertificateValidationCallback : ValidateServerCertificate | semmle.label | delegate creation of type RemoteCertificateValidationCallback : ValidateServerCertificate |
+| Program.cs:89:25:89:32 | access to local variable callback : ValidateServerCertificate | semmle.label | access to local variable callback : ValidateServerCertificate |
+| Program.cs:114:71:114:95 | delegate creation of type RemoteCertificateValidationCallback | semmle.label | delegate creation of type RemoteCertificateValidationCallback |
+subpaths
+#select
+| Program.cs:18:13:18:78 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:18:13:18:78 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:18:13:18:78 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:18:13:18:78 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:18:13:18:78 | delegate creation of type RemoteCertificateValidationCallback | here |
+| Program.cs:59:61:59:106 | (...) => ... | Program.cs:59:61:59:106 | (...) => ... | Program.cs:59:61:59:106 | (...) => ... | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:59:61:59:106 | (...) => ... | This certificate callback | Program.cs:59:61:59:106 | (...) => ... | here |
+| Program.cs:66:67:66:132 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:66:67:66:132 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:66:67:66:132 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:66:67:66:132 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:66:67:66:132 | delegate creation of type RemoteCertificateValidationCallback | here |
+| Program.cs:67:67:67:112 | (...) => ... | Program.cs:67:67:67:112 | (...) => ... | Program.cs:67:67:67:112 | (...) => ... | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:67:67:67:112 | (...) => ... | This certificate callback | Program.cs:67:67:67:112 | (...) => ... | here |
+| Program.cs:68:67:68:91 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:68:67:68:91 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:68:67:68:91 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:68:67:68:91 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:68:67:68:91 | delegate creation of type RemoteCertificateValidationCallback | here |
+| Program.cs:75:55:75:79 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:75:55:75:79 | delegate creation of type RemoteCertificateValidationCallback | 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:75:55:75:79 | delegate creation of type RemoteCertificateValidationCallback | here |
+| Program.cs:82:67:82:74 | access to parameter callback | Program.cs:87:56:87:80 | delegate creation of type RemoteCertificateValidationCallback : ValidateServerCertificate | Program.cs:82:67:82:74 | access to parameter callback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:82:67:82:74 | access to parameter callback | This certificate callback | Program.cs:87:56:87:80 | delegate creation of type RemoteCertificateValidationCallback : ValidateServerCertificate | here |
+| Program.cs:114:71:114:95 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:114:71:114:95 | delegate creation of type RemoteCertificateValidationCallback | Program.cs:114:71:114:95 | delegate creation of type RemoteCertificateValidationCallback | $@ that is defined $@ and accepts any certificate as valid, is used here. | Program.cs:114:71:114:95 | delegate creation of type RemoteCertificateValidationCallback | This certificate callback | Program.cs:114:71:114:95 | delegate creation of type RemoteCertificateValidationCallback | 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,122 @@
+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
+{
+
+    void M1()
+    {
+        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;
+    }
+
+    void M2()
+    {
+        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);
+    }
+
+    void M3()
+    {
+        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
+    }
+
+    void M4()
+    {
+        HttpWebRequest request = (HttpWebRequest)WebRequest.Create("https://www.example.com");
+        request.ServerCertificateValidationCallback = ValidateServerCertificate; // BAD: unsafe callback used
+        request.ServerCertificateValidationCallback = SafeValidateServerCertificate; // GOOD: safe callback used
+
+    }
+
+    void SetCallback(RemoteCertificateValidationCallback callback)
+    {
+        ServicePointManager.ServerCertificateValidationCallback = callback; // BAD: unsafe callback used
+    }
+
+    void M5(bool b)
+    {
+        RemoteCertificateValidationCallback callback = ValidateServerCertificate;
+        if (b) {
+            SetCallback(callback); // BAD: unsafe callback used
+        }
+    }
+
+    void M6(Settings settings)
+    {
+        RemoteCertificateValidationCallback callback = ValidateServerCertificate;
+        if (settings.IgnoreCertificateValidation)
+        {
+            SetCallback(callback); // GOOD: We don't do validation.
+        }
+    }
+
+    void M7(Settings settings)
+    {
+        if (settings.IgnoreCertificateValidation)
+        {
+            ServicePointManager.ServerCertificateValidationCallback = ValidateServerCertificate; // GOOD: We don't do validation.
+        }
+    }
+
+    void M8(Settings settings)
+    {
+        if (!settings.IgnoreCertificateValidation)
+        {
+            ServicePointManager.ServerCertificateValidationCallback = ValidateServerCertificate; // BAD: unsafe callback used
+        }
+    }
+}
+
+public class Settings {
+
+    public bool IgnoreCertificateValidation { get; set; }
+}

csharp/ql/test/experimental/CWE-295/options

@@ -0,0 +1,2 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj