Don't use weak password-based key derivation

Rule ND3123 Security

Matches methods creating PasswordDeriveBytes (PBKDF1, obsolete SYSLIB0041) or Rfc2898DeriveBytes through a constructor without a HashAlgorithmName parameter: such constructors silently use SHA1 and default to 1000 iterations, orders of magnitude below the OWASP 2023 recommendation (600.000 iterations for PBKDF2-HMAC-SHA256).

This is CWE-916 Use of Password Hash With Insufficient Computational Effort.

Flagged Code
static byte[] DeriveKey(string password, byte[] salt) {
   using var kdf = new Rfc2898DeriveBytes(password, salt, 1000);   // flagged: SHA1 default
   return kdf.GetBytes(32);
}
static byte[] LegacyKey(string password, byte[] salt) =>
   new PasswordDeriveBytes(password, salt).GetBytes(20);           // flagged: PBKDF1
Fixed Code
static byte[] DeriveKey(string password, byte[] salt) =>
   Rfc2898DeriveBytes.Pbkdf2(password, salt, 600_000, HashAlgorithmName.SHA256, 32);
// Or the constructor overload that names the hash algorithm explicitly:
static byte[] DeriveKeyCtor(string password, byte[] salt) {
   using var kdf = new Rfc2898DeriveBytes(password, salt, 600_000, HashAlgorithmName.SHA256);
   return kdf.GetBytes(32);
}

How to fix

Use Rfc2898DeriveBytes.Pbkdf2(password, salt, iterations, HashAlgorithmName.SHA256, length) (or the constructor overload taking a HashAlgorithmName) with an explicit high iteration count, or a memory-hard function such as Argon2id or scrypt from a maintained library. Store the iteration count with the hash so it can be raised over time.

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 weak password-based key derivation</Name>
// <Id>ND3123:DontUseWeakPasswordBasedKeyDerivation</Id>
warnif count > 0

// Third-party members are part of the code model only when the application uses them.
let weakKdfMembers =
   // PBKDF1 (PasswordDeriveBytes) is obsolete (SYSLIB0041): it produces at most 20 bytes and is not a standard KDF
   ThirdParty.Types.WithFullName("System.Security.Cryptography.PasswordDeriveBytes")
      .ChildMethods().Where(m => m.IsConstructor)
   // Rfc2898DeriveBytes constructors without a HashAlgorithmName parameter default to SHA1 (and 1000 iterations)
   .Concat(ThirdParty.Types.WithFullName("System.Security.Cryptography.Rfc2898DeriveBytes")
      .ChildMethods().Where(m => m.IsConstructor && !m.Name.Contains("HashAlgorithmName")))
   .ToHashSetEx()

where weakKdfMembers.Any()

from m in Application.Methods.UsingAny(weakKdfMembers)
let weakKdfMembersUsed = m.MethodsCalled.Intersect(weakKdfMembers).ToArray()
select new {
  m,
  weakKdfMembersUsed,
  Debt = 30.ToMinutes().ToDebt(),
  Severity = Severity.High
}

//<Expl>
// The *{0}* derives a key from a password with *{1}*, which relies on
// PBKDF1 or on PBKDF2-SHA1 with a default iteration count.
//</Expl>

//<Desc>
// Matches methods creating *PasswordDeriveBytes* (PBKDF1, obsolete
// SYSLIB0041) or *Rfc2898DeriveBytes* through a constructor without a
// *HashAlgorithmName* parameter: such constructors silently use SHA1 and
// default to 1000 iterations, orders of magnitude below the OWASP 2023
// recommendation (600.000 iterations for PBKDF2-HMAC-SHA256).
//
// This is CWE-916 *Use of Password Hash With Insufficient Computational
// Effort*.
//
// **Flagged Code**
// ```csharp
// static byte[] DeriveKey(string password, byte[] salt) {
//    using var kdf = new Rfc2898DeriveBytes(password, salt, 1000);   // flagged: SHA1 default
//    return kdf.GetBytes(32);
// }
// static byte[] LegacyKey(string password, byte[] salt) =>
//    new PasswordDeriveBytes(password, salt).GetBytes(20);           // flagged: PBKDF1
// ```
//
// **Fixed Code**
// ```csharp
// static byte[] DeriveKey(string password, byte[] salt) =>
//    Rfc2898DeriveBytes.Pbkdf2(password, salt, 600_000, HashAlgorithmName.SHA256, 32);
// // Or the constructor overload that names the hash algorithm explicitly:
// static byte[] DeriveKeyCtor(string password, byte[] salt) {
//    using var kdf = new Rfc2898DeriveBytes(password, salt, 600_000, HashAlgorithmName.SHA256);
//    return kdf.GetBytes(32);
// }
// ```
//</Desc>

//<HowToFix>
// Use *Rfc2898DeriveBytes.Pbkdf2(password, salt, iterations, HashAlgorithmName.SHA256, length)*
// (or the constructor overload taking a *HashAlgorithmName*) with an
// explicit high iteration count, or a memory-hard function such as
// *Argon2id* or *scrypt* from a maintained library. Store the iteration
// count with the hash so it can be raised over time.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items