Merge pull request github#6425 from raulgarciamsft/insecureRandom_potential_fix

C#: Adding Membership.GeneratePassword() as a bad source of random data

Author: tamasvajk

csharp/change-notes/2021-08-05-insecure-randomness.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* Membership.GeneratePassword()` has been added as a bad source of random data.

csharp/ql/src/Security Features/InsecureRandomness.cs

@@ -15,7 +15,7 @@ string GeneratePassword()
         password = "mypassword" + BitConverter.ToInt32(randomBytes);
     }
 
-    // GOOD: Password is generated using a cryptographically secure RNG
+    // BAD: Membership.GeneratePassword generates a password with a bias
     password = Membership.GeneratePassword(12, 3);
 
     return password;

csharp/ql/src/Security Features/InsecureRandomness.ql

@@ -59,6 +59,13 @@ module Random {
       this.getExpr() =
         any(MethodCall mc |
           mc.getQualifier().getType().(RefType).hasQualifiedName("System", "Random")
+          or
+          // by using `% 87` on a `byte`, `System.Web.Security.Membership.GeneratePassword` has a bias
+          mc.getQualifier()
+              .getType()
+              .(RefType)
+              .hasQualifiedName("System.Web.Security", "Membership") and
+          mc.getTarget().hasName("GeneratePassword")
         )
     }
   }

csharp/ql/test/query-tests/Security Features/CWE-338/InsecureRandomness.cs

@@ -73,4 +73,24 @@ public static string InsecureRandomStringFromIndexer(int length)
         }
         return result;
     }
+
+    public static string BiasPasswordGeneration()
+    {
+        // BAD: Membership.GeneratePassword generates a password with a bias
+        string password =  System.Web.Security.Membership.GeneratePassword(12, 3);
+        return password;
+    }
+
+}
+
+namespace System.Web.Security
+{
+    public static class Membership
+    {
+        public static string GeneratePassword(int length, int numberOfNonAlphanumericCharacters)
+        {
+            return "stub";
+        }
+
+    }
 }

csharp/ql/test/query-tests/Security Features/CWE-338/InsecureRandomness.expected

@@ -29,7 +29,9 @@ nodes
 | InsecureRandomness.cs:62:16:62:32 | call to method ToString : String | semmle.label | call to method ToString : String |
 | InsecureRandomness.cs:72:31:72:39 | call to method Next : Int32 | semmle.label | call to method Next : Int32 |
 | InsecureRandomness.cs:74:16:74:21 | access to local variable result : String | semmle.label | access to local variable result : String |
+| InsecureRandomness.cs:80:28:80:81 | call to method GeneratePassword | semmle.label | call to method GeneratePassword |
 #select
 | InsecureRandomness.cs:12:27:12:50 | call to method InsecureRandomString | InsecureRandomness.cs:28:29:28:43 | call to method Next : Int32 | InsecureRandomness.cs:12:27:12:50 | call to method InsecureRandomString | Cryptographically insecure random number is generated at $@ and used here in a security context. | InsecureRandomness.cs:28:29:28:43 | call to method Next | call to method Next |
 | InsecureRandomness.cs:13:20:13:56 | call to method InsecureRandomStringFromSelection | InsecureRandomness.cs:60:31:60:39 | call to method Next : Int32 | InsecureRandomness.cs:13:20:13:56 | call to method InsecureRandomStringFromSelection | Cryptographically insecure random number is generated at $@ and used here in a security context. | InsecureRandomness.cs:60:31:60:39 | call to method Next | call to method Next |
 | InsecureRandomness.cs:14:20:14:54 | call to method InsecureRandomStringFromIndexer | InsecureRandomness.cs:72:31:72:39 | call to method Next : Int32 | InsecureRandomness.cs:14:20:14:54 | call to method InsecureRandomStringFromIndexer | Cryptographically insecure random number is generated at $@ and used here in a security context. | InsecureRandomness.cs:72:31:72:39 | call to method Next | call to method Next |
+| InsecureRandomness.cs:80:28:80:81 | call to method GeneratePassword | InsecureRandomness.cs:80:28:80:81 | call to method GeneratePassword | InsecureRandomness.cs:80:28:80:81 | call to method GeneratePassword | Cryptographically insecure random number is generated at $@ and used here in a security context. | InsecureRandomness.cs:80:28:80:81 | call to method GeneratePassword | call to method GeneratePassword |