diff --git a/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.qhelp b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.qhelp new file mode 100644 index 0000000..0c55f17 --- /dev/null +++ b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.qhelp @@ -0,0 +1,44 @@ + + + +

+Setting ServicePointManager.ServerCertificateValidationCallback to a callback that always returns +true disables SSL/TLS certificate validation globally for all HTTPS requests in the application. +This makes the application vulnerable to man-in-the-middle (MITM) attacks, as it will accept any SSL certificate, +including invalid, expired, or malicious ones. +

+

+When certificate validation is disabled, an attacker can intercept HTTPS communications, potentially gaining access +to sensitive data such as credentials, session tokens, or personal information. +

+
+ + +

+Never disable SSL certificate validation globally. Instead: +

+ +
+ + +

The following example shows insecure code that disables certificate validation globally:

+ + +

The following example shows secure alternatives:

+ +
+ + +
  • OWASP: Improper Certificate Validation
  • +
  • CWE: CWE-295: Improper Certificate Validation
  • +
  • Microsoft Docs: ServicePointManager.ServerCertificateValidationCallback
  • +
    +
    diff --git a/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.ql b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.ql new file mode 100644 index 0000000..518403e --- /dev/null +++ b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.ql @@ -0,0 +1,20 @@ +/** + * @name Global SSL certificate validation disabled + * @description Setting ServicePointManager.ServerCertificateValidationCallback to always return true + * disables SSL certificate validation globally, making the application vulnerable to + * man-in-the-middle attacks. + * @kind problem + * @problem.severity error + * @security-severity 8.1 + * @precision high + * @id csharp/web/insecure-ssl-validation + * @tags security + * external/cwe/cwe-295 + */ + +import csharp +import GlobalSslDisable + +from GlobalSslDisable disable +select disable, + "Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks." diff --git a/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.qll b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.qll new file mode 100644 index 0000000..1fbe69b --- /dev/null +++ b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisable.qll @@ -0,0 +1,107 @@ +/** + * Provides classes and predicates for detecting globally disabled SSL certificate validation. + */ + +import csharp + +/** + * A property access to `ServicePointManager.ServerCertificateValidationCallback`. + */ +class ServerCertificateValidationCallbackAccess extends PropertyAccess { + ServerCertificateValidationCallbackAccess() { + this.getTarget().getName() = "ServerCertificateValidationCallback" and + this.getTarget().getDeclaringType().hasFullyQualifiedName("System.Net", "ServicePointManager") + } +} + +/** + * An assignment or compound assignment to `ServicePointManager.ServerCertificateValidationCallback`. + */ +class ServerCertificateValidationCallbackAssignment extends AssignExpr { + ServerCertificateValidationCallbackAssignment() { + this.getLValue() instanceof ServerCertificateValidationCallbackAccess + } + + Expr getRightHandSide() { result = this.getRValue() } +} + +/** + * A compound assignment (+=) to `ServicePointManager.ServerCertificateValidationCallback`. + */ +class ServerCertificateValidationCallbackAddition extends AssignAddExpr { + ServerCertificateValidationCallbackAddition() { + this.getLValue() instanceof ServerCertificateValidationCallbackAccess + } + + Expr getRightHandSide() { result = this.getRValue() } +} + +/** + * Holds if the method always returns true without any conditional logic. + */ +predicate methodAlwaysReturnsTrue(Method m) { + // Method with expression body: => true + m.getExpressionBody() instanceof BoolLiteral and + m.getExpressionBody().(BoolLiteral).getValue() = "true" + or + // Method with single return statement: return true; + exists(ReturnStmt ret | + m.getStatementBody().(BlockStmt).getNumberOfStmts() = 1 and + ret = m.getStatementBody().(BlockStmt).getStmt(0) and + ret.getExpr() instanceof BoolLiteral and + ret.getExpr().(BoolLiteral).getValue() = "true" + ) +} + +/** + * An expression that always returns true. + */ +predicate alwaysReturnsTrue(Expr e) { + // Simple lambda: (args) => true + exists(LambdaExpr lambda | + e = lambda and + lambda.getExpressionBody() instanceof BoolLiteral and + lambda.getExpressionBody().(BoolLiteral).getValue() = "true" + ) + or + // Lambda with block body containing only "return true;" + exists(LambdaExpr lambda, ReturnStmt ret | + e = lambda and + lambda.getStatementBody().(BlockStmt).getNumberOfStmts() = 1 and + ret = lambda.getStatementBody().(BlockStmt).getStmt(0) and + ret.getExpr() instanceof BoolLiteral and + ret.getExpr().(BoolLiteral).getValue() = "true" + ) + or + // Method reference (as callable access) that always returns true + exists(Method m | + e.(CallableAccess).getTarget() = m and + methodAlwaysReturnsTrue(m) + ) + or + // Method access (member access) that references a method always returning true + exists(Method m | + e.(MemberAccess).getTarget() = m and + methodAlwaysReturnsTrue(m) + ) + or + // Delegate creation (implicit or explicit) from a method that always returns true + exists(Method m | + e.(DelegateCreation).getArgument() = m.getAnAccess() and + methodAlwaysReturnsTrue(m) + ) +} + +/** + * An assignment or addition to ServerCertificateValidationCallback that always returns true, + * thereby disabling SSL certificate validation globally. + */ +class GlobalSslDisable extends Expr { + GlobalSslDisable() { + this instanceof ServerCertificateValidationCallbackAssignment and + alwaysReturnsTrue(this.(ServerCertificateValidationCallbackAssignment).getRightHandSide()) + or + this instanceof ServerCertificateValidationCallbackAddition and + alwaysReturnsTrue(this.(ServerCertificateValidationCallbackAddition).getRightHandSide()) + } +} diff --git a/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisableBad.cs b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisableBad.cs new file mode 100644 index 0000000..c4b7e49 --- /dev/null +++ b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisableBad.cs @@ -0,0 +1,12 @@ +using System.Net; +using System.Net.Security; + +class Example +{ + void Bad() + { + // BAD: Disables SSL certificate validation globally for all HTTPS requests + ServicePointManager.ServerCertificateValidationCallback += + (sender, certificate, chain, sslPolicyErrors) => true; + } +} diff --git a/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisableGood.cs b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisableGood.cs new file mode 100644 index 0000000..5121e26 --- /dev/null +++ b/languages/csharp/custom/src/GlobalSslDisable/GlobalSslDisableGood.cs @@ -0,0 +1,48 @@ +using System; +using System.Net; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; + +class Example +{ + void Good1() + { + // GOOD: Use default certificate validation (don't set the callback) + var request = WebRequest.Create("https://example.com"); + } + + void Good2() + { + // GOOD: Implement proper certificate validation logic + ServicePointManager.ServerCertificateValidationCallback += + (sender, certificate, chain, sslPolicyErrors) => + { + if (sslPolicyErrors == SslPolicyErrors.None) + { + return true; + } + + // Log the error and reject the certificate + Console.WriteLine("Certificate error: {0}", sslPolicyErrors); + return false; + }; + } + + void Good3() + { + // GOOD: Validate specific certificate properties (certificate pinning) + var trustedThumbprints = new[] { "ABC123DEF456..." }; + + ServicePointManager.ServerCertificateValidationCallback += + (sender, certificate, chain, sslPolicyErrors) => + { + var cert = certificate as X509Certificate2; + if (cert != null && Array.IndexOf(trustedThumbprints, cert.Thumbprint) >= 0) + { + return true; + } + + return sslPolicyErrors == SslPolicyErrors.None; + }; + } +} diff --git a/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.cs b/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.cs new file mode 100644 index 0000000..4be4ed7 --- /dev/null +++ b/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.cs @@ -0,0 +1,121 @@ +using System; + +// Mock types for testing purposes (since System.Net types are not available in test environment) +namespace System.Net +{ + public static class ServicePointManager + { + public static RemoteCertificateValidationCallback ServerCertificateValidationCallback { get; set; } + } + + public delegate bool RemoteCertificateValidationCallback(object sender, object certificate, object chain, object sslPolicyErrors); +} + +namespace GlobalSslDisableTests +{ + public class GlobalSslDisableTest + { + // NON_COMPLIANT: Lambda that always returns true - disables all SSL validation + public void DisableAllSslValidationWithLambda() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback += (sender, certificate, chain, sslPolicyErrors) => true; + } + + // NON_COMPLIANT: Lambda with explicit return true - disables all SSL validation + public void DisableAllSslValidationWithExplicitReturn() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback += (sender, certificate, chain, sslPolicyErrors) => + { + return true; + }; + } + + // NON_COMPLIANT: Direct assignment to always return true + public void DisableAllSslValidationWithAssignment() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true; + } + + // NON_COMPLIANT: Delegate method that always returns true + public void DisableAllSslValidationWithDelegateMethod() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback += AlwaysAcceptCertificate; + } + + private static bool AlwaysAcceptCertificate(object sender, object certificate, object chain, object sslPolicyErrors) + { + return true; + } + + // NON_COMPLIANT: Another delegate that always returns true + public void DisableAllSslValidationWithAnotherDelegate() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback = AcceptAllCerts; + } + + private static bool AcceptAllCerts(object sender, object cert, object chain, object errors) + { + return true; + } + + // NON_COMPLIANT: Expression-bodied method that returns true + private static bool AlwaysTrueExpressionBody(object sender, object certificate, object chain, object sslPolicyErrors) => true; + + public void DisableWithExpressionBodyDelegate() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback = AlwaysTrueExpressionBody; + } + + // COMPLIANT: Lambda with conditional logic that actually validates certificates + public void ValidateCertificateWithCondition() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback += (sender, certificate, chain, sslPolicyErrors) => + { + var x = 5; + if (x > 0) + { + return true; + } + return false; + }; + } + + // COMPLIANT: Null assignment (resetting the callback) + public void ResetCertificateValidation() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback = null; + } + + // COMPLIANT: Delegate method with actual validation logic + public void DelegateWithValidation() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback += ValidateCertificate; + } + + private static bool ValidateCertificate(object sender, object certificate, object chain, object sslPolicyErrors) + { + var y = 10; + if (y > 5) + { + return true; + } + return false; + } + + // COMPLIANT: Lambda that returns false (rejects all - not a security issue) + public void RejectAllCertificates() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => false; + } + + // COMPLIANT: Lambda with multiple statements but conditional return + public void ConditionalValidation() + { + System.Net.ServicePointManager.ServerCertificateValidationCallback = (sender, cert, chain, errors) => + { + var test = 42; + return test > 0; + }; + } + } +} diff --git a/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.expected b/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.expected new file mode 100644 index 0000000..46da88a --- /dev/null +++ b/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.expected @@ -0,0 +1,6 @@ +| GlobalSslDisable.cs:21:13:21:135 | ... += ... | Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks. | +| GlobalSslDisable.cs:27:13:30:13 | ... += ... | Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks. | +| GlobalSslDisable.cs:36:13:36:134 | ... = ... | Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks. | +| GlobalSslDisable.cs:42:13:42:105 | ... += ... | Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks. | +| GlobalSslDisable.cs:53:13:53:95 | ... = ... | Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks. | +| GlobalSslDisable.cs:66:13:66:105 | ... = ... | Globally disabling SSL certificate validation makes the application vulnerable to man-in-the-middle attacks. | diff --git a/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.qlref b/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.qlref new file mode 100644 index 0000000..b915a26 --- /dev/null +++ b/languages/csharp/custom/test/GlobalSslDisable/GlobalSslDisable.qlref @@ -0,0 +1 @@ +GlobalSslDisable/GlobalSslDisable.ql \ No newline at end of file