CS: Adding DecryptWithoutHash and CertificateValidationDisabled queries

csharp/ql/src/Security Features/CWE-295/CertificateValidationDisabled.ql

@@ -0,0 +1,50 @@
+/**
+ * @name Do not disable certificate validation
+ * @description Do not force ServerCertificateValidationCallback to always return 'true'.
+ * @kind problem
+ * @id cs/do-not-disable-cert-validation
+ * @problem.severity error
+ * @precision high
+ * @tags security
+ *       external/cwe/cwe-295
+ */
+import csharp
+/*
+ * Lambda examples:
+ *  ServicePointManager.ServerCertificateValidationCallback += (a, b, c, d) => { return true; };
+ *  ServicePointManager.ServerCertificateValidationCallback += (a, b, c, d) => true;
+ * Anonymous method example:
+ *  ServicePointManager.ServerCertificateValidationCallback += delegate { return true; };
+ * Delegate creation example:
+ *  ServicePointManager.ServerCertificateValidationCallback = new RemoteCertificateValidationCallback(AcceptAllCertifications);
+ */
+from Assignment a, PropertyWrite leftExpr, Callable targetCallable, Property p
+where leftExpr = a.getLValue()  // Hook up the basic fields
+    and leftExpr.getTarget() = p
+    and p.getDeclaringType().hasQualifiedName("System.Net", "ServicePointManager")
+    and p.getName() = "ServerCertificateValidationCallback"
+    // Find the target callable being assigned to the ServerCertificateValidationCallback
+    and exists(Expr assignedValue |
+        assignedValue = a.getRValue() |
+        // Either the assigned value is an AnonymousFunctionExpr, such as a lambda or anonymous method expr
+        targetCallable = assignedValue or
+        // Or the assigned value is a DelegateCreation
+        exists(Expr delegateCreationArg |
+            delegateCreationArg = assignedValue.(DelegateCreation).getArgument() |
+            // And the argument is an AnonymousFunctionExpr
+            targetCallable = delegateCreationArg or
+            // Or the argument is an access to a callable
+            targetCallable = delegateCreationArg.(CallableAccess).getTarget()
+        )
+    )
+    // If the target callable returns true, validation is disabled
+    and(
+        exists(ReturnStmt rs, BlockStmt bs |
+            bs = targetCallable.getBody() and
+            bs.getNumberOfChildren() = 1 and
+            bs.getChild(0) = rs and
+            rs.getExpr().(BoolLiteral).getBoolValue() = true
+        ) or
+        targetCallable.getBody().(BoolLiteral).getBoolValue() = true
+    )
+select a, "Certificate validation disabled."

csharp/ql/src/Security Features/CWE-327/DecryptWithoutHash.ql

@@ -0,0 +1,64 @@
+/**
+ * @name Verify hash before decryption
+ * @description 
+ * @kind problem
+ * @id cs/verify-hash-before-decryption
+ * @problem.severity error
+ * @precision medium
+ * @tags security
+ */
+import csharp
+import semmle.code.csharp.dataflow.TaintTracking::TaintTracking
+
+class CryptoStreamWrite extends MethodCall {
+  CryptoStreamWrite() {
+    getTarget().hasQualifiedName("System.Security.Cryptography.CryptoStream", "Write")
+  }
+
+  /** Holds if this is a decryptor stream. */
+  predicate isDecryptor() {
+    any(CryptoStreamTaintTrackingConfig c).hasFlow(_, DataFlow::exprNode(getQualifier()))
+  }
+}
+
+class ComputeHash extends MethodCall {
+  ComputeHash() {
+    getTarget().hasName("ComputeHash")
+  }
+}
+
+class ByteArrayTaintTrackingConfig extends TaintTracking::Configuration  {
+  ByteArrayTaintTrackingConfig() { this = "Byte array that is being passed to cryptostream write" }
+
+  override predicate isSource(DataFlow::Node source) {    
+    source.asExpr().getType().(ArrayType).getElementType().hasName("Byte")
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink.asExpr() = any(CryptoStreamWrite cw).getArgument(0) or
+    sink.asExpr() = any(ComputeHash ch).getArgument(0)
+  }
+}
+
+class CryptoStreamTaintTrackingConfig extends TaintTracking::Configuration  {
+  CryptoStreamTaintTrackingConfig() { this = "CryptoStreamTaintTracking" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(ObjectCreation oc |
+      oc = source.asExpr() and
+      oc.getTarget().getDeclaringType().hasName("CryptoStream") and
+      oc.getArgument(1).(MethodCall).getTarget().hasQualifiedName("System.Security.Cryptography.SymmetricAlgorithm", "CreateDecryptor")
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink.asExpr() = any(CryptoStreamWrite csw).getQualifier()
+  }
+}
+
+from ByteArrayTaintTrackingConfig config, Expr arrayDefinition, CryptoStreamWrite writeCall
+where config.hasFlow(DataFlow::exprNode(arrayDefinition), DataFlow::exprNode(writeCall.getArgument(0)))
+  and writeCall.isDecryptor()
+  and not exists(ComputeHash c | config.hasFlow(DataFlow::exprNode(arrayDefinition), DataFlow::exprNode(c.getArgument(0))))
+select arrayDefinition, "Byte array passed to CryptoStream $@, never has the hash computed.", arrayDefinition, "here"
+

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

@@ -0,0 +1,7 @@
+| Test.cs:20:13:20:129 | ... += ... | Certificate validation disabled. |
+| Test.cs:21:13:21:103 | ... += ... | Certificate validation disabled. |
+| Test.cs:22:13:22:91 | ... += ... | Certificate validation disabled. |
+| Test.cs:23:13:23:96 | ... += ... | Certificate validation disabled. |
+| Test.cs:26:13:26:134 | ... = ... | Certificate validation disabled. |
+| Test.cs:29:13:29:145 | ... = ... | Certificate validation disabled. |
+| Test.cs:69:13:69:148 | ... = ... | Certificate validation disabled. |

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

@@ -0,0 +1 @@
+Security Features/CWE-295/CertificateValidationDisabled.ql

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

@@ -0,0 +1,82 @@
+// semmle-extractor-options: /nostdlib /noconfig /r:${env.windir}\Microsoft.NET\Framework64\v4.0.30319\mscorlib.dll /r:${env.windir}\Microsoft.NET\Framework64\v4.0.30319\System.dll /r:${env.windir}\Microsoft.NET\Framework64\v4.0.30319\System.Net.dll /r:${env.windir}\Microsoft.NET\Framework64\v4.0.30319\System.Net.Http.dll /r:${env.windir}\Microsoft.NET\Framework64\v4.0.30319\System.Linq.dll
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Net;
+using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Net.Security;
+using System.Security.Cryptography.X509Certificates;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace SemmleTestCertValidation
+{
+    class Program
+    {
+        static void Main(string[] args)
+        {
+            // Bad (Simple)
+            ServicePointManager.ServerCertificateValidationCallback += (sender, cert, chain, sslPolicyErrors) => { return true; };
+            ServicePointManager.ServerCertificateValidationCallback += (a, b, c, d) => { return true; };
+            ServicePointManager.ServerCertificateValidationCallback += (a, b, c, d) => true;
+            ServicePointManager.ServerCertificateValidationCallback += delegate { return true; };
+
+            // Bad (Simple Function)
+            ServicePointManager.ServerCertificateValidationCallback = new RemoteCertificateValidationCallback(AcceptAllCertifications);
+
+            var m = new Program();
+            ServicePointManager.ServerCertificateValidationCallback = new RemoteCertificateValidationCallback(m.AcceptAllCertificationsNonStatic);
+            m.Test();
+
+            // Don't know
+            ServicePointManager.ServerCertificateValidationCallback += (a, b, c, d) => {
+                int x = 20;
+                if (x < 100)
+                {
+                    return false;
+                }
+                else
+                {
+                    return true;
+                }
+            };
+
+            // Good
+            ServicePointManager.ServerCertificateValidationCallback += delegate (
+                object sender,
+                X509Certificate cert,
+                X509Chain chain,
+                SslPolicyErrors sslPolicyErrors)
+            {
+                if (cert == null)
+                {
+                    return false;
+                }
+
+                if (sslPolicyErrors != SslPolicyErrors.None)
+                {
+                    return false;
+                }
+
+                return true;
+            };
+        }
+
+        void Test()
+        {
+            // BAD
+            ServicePointManager.ServerCertificateValidationCallback = new RemoteCertificateValidationCallback(this.AcceptAllCertificationsNonStatic);
+        }
+
+        public static bool AcceptAllCertifications(object sender, X509Certificate certification, X509Chain chain, SslPolicyErrors sslPolicyErrors)
+        {
+            return true;
+        }
+
+        public bool AcceptAllCertificationsNonStatic(object sender, X509Certificate certification, X509Chain chain, SslPolicyErrors sslPolicyErrors)
+        {
+            return true;
+        }
+    }
+}

csharp/ql/test/query-tests/Security Features/CWE-327/DecryptWithoutHash/DecryptWithoutHash.expected

@@ -0,0 +1 @@
+| Test.cs:156:31:156:40 | access to parameter cipherText | Byte array passed to CryptoStream $@, never has the hash computed. | Test.cs:156:31:156:40 | access to parameter cipherText | here |

csharp/ql/test/query-tests/Security Features/CWE-327/DecryptWithoutHash/DecryptWithoutHash.qlref

@@ -0,0 +1 @@
+Security Features/CWE-327/DecryptWithoutHash.ql

csharp/ql/test/query-tests/Security Features/CWE-327/DecryptWithoutHash/Test.cs

@@ -0,0 +1,177 @@
+// semmle-extractor-options: /r:System.Security.Cryptography.Algorithms.dll /r:System.Security.Cryptography.Primitives.dll
+using System;
+using System.IO;
+using System.Security.Cryptography;
+using System.Text;
+namespace EncryptWithoutHash
+{
+    public class Class1
+    {
+        int Rfc2898KeygenIterations = 100;
+        int AesKeySizeInBits = 128;
+        byte[] Salt = new byte[] { 0x15, 0xA0, 0xC3, 0x86, 0x52, 0x03, 0xF2, 0xFB, 0xD2, 0x99, 0xEA, 0xBC, 0x3D, 0xBB, 0xC1, 0xF8 };
+
+        byte[] hmacKey = new byte[] { 0x3A, 0x6C, 0xC8, 0x4E, 0x36, 0x1D, 0x15, 0x4A, 0x93, 0xBB, 0xD5, 0x49, 0xBB, 0x0D, 0x33, 0x15, 0xA0, 0xC3, 0x86, 0x52, 0x03, 0xF2, 0xFB, 0xD2, 0x99, 0xEA, 0xBC, 0x3D, 0xBB, 0xC1, 0xF8, 0x53,
+                                      0x3A, 0x6C, 0xC8, 0x4E, 0x36, 0x1D, 0x15, 0x4A, 0x93, 0xBB, 0xD5, 0x49, 0xBB, 0x0D, 0x33, 0x15, 0xA0, 0xC3, 0x86, 0x52, 0x03, 0xF2, 0xFB, 0xD2, 0x99, 0xEA, 0xBC, 0x3D, 0xBB, 0xC1, 0xF8, 0x53};
+
+        public byte[] Enrycpt(string plainText, string password)
+        {
+            byte[] rawPlaintext = Encoding.Unicode.GetBytes(plainText);
+            byte[] cipherText = null;
+            byte[] hmac;
+            using (Aes aes = new AesManaged())
+            {
+                aes.Padding = PaddingMode.PKCS7;
+                aes.Mode = CipherMode.CBC;
+                aes.KeySize = AesKeySizeInBits;
+                int KeyStrengthInBytes = aes.KeySize / 8;
+                Rfc2898DeriveBytes rfc2898 = new Rfc2898DeriveBytes(password, Salt, Rfc2898KeygenIterations);
+                aes.Key = rfc2898.GetBytes(KeyStrengthInBytes);
+                aes.IV = rfc2898.GetBytes(KeyStrengthInBytes);
+                using (MemoryStream ms = new MemoryStream())
+                {
+                    using (CryptoStream cs = new CryptoStream(ms, aes.CreateEncryptor(), CryptoStreamMode.Write))
+                    {
+                        cs.Write(rawPlaintext, 0, rawPlaintext.Length);
+                    }
+
+                    cipherText = ms.ToArray();
+                    HMACSHA256 hmacsha256 = new HMACSHA256(hmacKey);
+                    hmac = hmacsha256.ComputeHash(cipherText);
+                }
+
+                Console.WriteLine("HMACSHA256: {0}", BytesToString(hmac));
+                return cipherText;
+            }
+        }
+
+        public byte[] EnrycptHashExternal(string plainText, string password)
+        {
+            byte[] rawPlaintext = Encoding.Unicode.GetBytes(plainText);
+            byte[] cipherText = null;
+            byte[] hmac;
+            using (Aes aes = new AesManaged())
+            {
+                aes.Padding = PaddingMode.PKCS7;
+                aes.Mode = CipherMode.CBC;
+                aes.KeySize = AesKeySizeInBits;
+                int KeyStrengthInBytes = aes.KeySize / 8;
+                Rfc2898DeriveBytes rfc2898 = new Rfc2898DeriveBytes(password, Salt, Rfc2898KeygenIterations);
+                aes.Key = rfc2898.GetBytes(KeyStrengthInBytes);
+                aes.IV = rfc2898.GetBytes(KeyStrengthInBytes);
+                using (MemoryStream ms = new MemoryStream())
+                {
+                    using (CryptoStream cs = new CryptoStream(ms, aes.CreateEncryptor(), CryptoStreamMode.Write))
+                    {
+                        cs.Write(rawPlaintext, 0, rawPlaintext.Length);
+                    }
+
+                    cipherText = ms.ToArray();
+                    hmac = GetHash(cipherText);
+                }
+
+                Console.WriteLine("HMACSHA256: {0}", BytesToString(hmac));
+                return cipherText;
+            }
+        }
+
+        public byte[] EnrycptWithoutHash(string plainText, string password)
+        {
+            byte[] rawPlaintext = Encoding.Unicode.GetBytes(plainText);
+            byte[] cipherText = null;
+            using (Aes aes = new AesManaged())
+            {
+                aes.Padding = PaddingMode.PKCS7;
+                aes.Mode = CipherMode.CBC;
+                aes.KeySize = AesKeySizeInBits;
+                int KeyStrengthInBytes = aes.KeySize / 8;
+                Rfc2898DeriveBytes rfc2898 = new Rfc2898DeriveBytes(password, Salt, Rfc2898KeygenIterations);
+                aes.Key = rfc2898.GetBytes(KeyStrengthInBytes);
+                aes.IV = rfc2898.GetBytes(KeyStrengthInBytes);
+                using (MemoryStream ms = new MemoryStream())
+                {
+                    using (CryptoStream cs = new CryptoStream(ms, aes.CreateEncryptor(), CryptoStreamMode.Write))
+                    {
+                        cs.Write(rawPlaintext, 0, rawPlaintext.Length);
+                    }
+
+                    cipherText = ms.ToArray();
+                }
+
+                return cipherText;
+            }
+        }
+
+        public byte[] GetHash(byte[] input)
+        {
+            HMACSHA256 hmacsha256 = new HMACSHA256(hmacKey);
+            return hmacsha256.ComputeHash(input);
+        }
+
+        public string Decrypt(byte[] cipherText, string password)
+        {
+            byte[] rawPlaintext;
+            byte[] hmac;
+            using (Aes aes = new AesManaged())
+            {
+                aes.Padding = PaddingMode.PKCS7;
+                aes.KeySize = AesKeySizeInBits;
+                int KeyStrengthInBytes = aes.KeySize / 8;
+                Rfc2898DeriveBytes rfc2898 = new Rfc2898DeriveBytes(password, Salt, Rfc2898KeygenIterations);
+                aes.Key = rfc2898.GetBytes(KeyStrengthInBytes);
+                aes.IV = rfc2898.GetBytes(KeyStrengthInBytes);
+                using (MemoryStream ms = new MemoryStream())
+                {
+                    using (CryptoStream cs = new CryptoStream(ms, aes.CreateDecryptor(), CryptoStreamMode.Write))
+                    {
+                        cs.Write(cipherText, 0, cipherText.Length);
+                    }
+
+                    HMACSHA256 hmacsha256 = new HMACSHA256(hmacKey);
+                    hmac = hmacsha256.ComputeHash(cipherText);
+
+                    rawPlaintext = ms.ToArray();
+                }
+
+                Console.WriteLine("HMACSHA256: {0}", BytesToString(hmac));
+                return Encoding.Unicode.GetString(rawPlaintext);
+            }
+        }
+
+        public string DecryptWithoutHash(byte[] cipherText, string password)
+        {
+            byte[] rawPlaintext;
+            using (Aes aes = new AesManaged())
+            {
+                aes.Padding = PaddingMode.PKCS7;
+                aes.KeySize = AesKeySizeInBits;
+                int KeyStrengthInBytes = aes.KeySize / 8;
+                Rfc2898DeriveBytes rfc2898 = new Rfc2898DeriveBytes(password, Salt, Rfc2898KeygenIterations);
+                aes.Key = rfc2898.GetBytes(KeyStrengthInBytes);
+                aes.IV = rfc2898.GetBytes(KeyStrengthInBytes);
+                using (MemoryStream ms = new MemoryStream())
+                {
+                    using (CryptoStream cs = new CryptoStream(ms, aes.CreateDecryptor(), CryptoStreamMode.Write))
+                    {
+                        cs.Write(cipherText, 0, cipherText.Length);
+                    }
+
+                    rawPlaintext = ms.ToArray();
+                }
+
+                return Encoding.Unicode.GetString(rawPlaintext);
+            }
+        }
+
+        public string BytesToString(byte[] byteArray)
+        {
+            StringBuilder str = new StringBuilder();
+            foreach (byte b in byteArray)
+            {
+                str.Append(b.ToString("X2"));
+            }
+
+            return str.ToString();
+        }
+    }
+}