Don't use System.Random for security purposes

Rule ND3101 Security

System.Random is weak: its numbers can be predicted, which can lead to vulnerabilities in a security critical context (tokens, session ids, salts, password reset codes, CAPTCHA...).

This is CWE-338 Use of Cryptographically Weak Pseudo-Random Number Generator.

Flagged Code
string CreatePasswordResetCode() {
   var random = new Random();   // flagged: predictable generator
   return random.Next(100000, 999999).ToString();
}

string CreateSessionId() => Random.Shared.NextInt64().ToString("x");   // flagged: Random.Shared
Fixed Code
string CreatePasswordResetCode() =>
   RandomNumberGenerator.GetInt32(100000, 1000000).ToString();

string CreateSessionId() => RandomNumberGenerator.GetHexString(32);

How to fix

In a security critical context use System.Security.Cryptography.RandomNumberGenerator (GetInt32(), GetBytes(), GetHexString(), GetString()): slower, but unpredictable. See: https://stackoverflow.com/questions/32932679/using-rngcryptoserviceprovider-to-generate-random-string Otherwise keep the faster System.Random and suppress the issues. More on System.Random weakness: https://stackoverflow.com/a/6842191/27194

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Don't use System.Random for security purposes</Name>
// <Id>ND3101:DontUseSystemRandomForSecurityPurposes</Id>
warnif count > 0

from m in Application.Methods
where m.CreateA("System.Random".AllowNoMatch()) ||
      m.IsUsing("System.Random.get_Shared()".AllowNoMatch())
select new {
  m,
  Debt = 15.ToMinutes().ToDebt(),
  AnnualInterest = 1.ToHours().ToAnnualInterest()
}

//<Expl>
// The *{0}* should not use *System.Random* (nor *Random.Shared*) because it is a
// *weak* pseudo-random number generator.
// This can potentially allow an attacker to predict security-sensitive
// values that are generated.
//</Expl>

//<Desc>
// **System.Random** is weak: its numbers can be predicted, which can lead
// to vulnerabilities in a security critical context (tokens, session ids,
// salts, password reset codes, CAPTCHA...).
//
// This is CWE-338 *Use of Cryptographically Weak Pseudo-Random Number Generator*.
//
// **Flagged Code**
// ```csharp
// string CreatePasswordResetCode() {
//    var random = new Random();   // flagged: predictable generator
//    return random.Next(100000, 999999).ToString();
// }
//
// string CreateSessionId() => Random.Shared.NextInt64().ToString("x");   // flagged: Random.Shared
// ```
//
// **Fixed Code**
// ```csharp
// string CreatePasswordResetCode() =>
//    RandomNumberGenerator.GetInt32(100000, 1000000).ToString();
//
// string CreateSessionId() => RandomNumberGenerator.GetHexString(32);
// ```
//</Desc>

//<HowToFix>
// In a security critical context use
// **System.Security.Cryptography.RandomNumberGenerator** (*GetInt32()*,
// *GetBytes()*, *GetHexString()*, *GetString()*): slower, but unpredictable.
// See: https://stackoverflow.com/questions/32932679/using-rngcryptoserviceprovider-to-generate-random-string
// Otherwise keep the faster **System.Random** and suppress the issues.
// More on *System.Random* weakness: https://stackoverflow.com/a/6842191/27194
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items