Review secrets compared with non-constant-time equality

Rule ND3113 Security

Matches methods that compare strings or byte sequences with String.Equals(), ==, String.Compare() or SequenceEqual() and either compute a hash / MAC / derived key in the same body, or carry a name mentioning a token, signature, HMAC, secret, OTP, password, API key, digest or nonce. Ordinary equality short-circuits on the first mismatch, so the elapsed time leaks how many leading bytes were right: a timing side channel that has been exploited over networks.

This is CWE-208 Observable Timing Discrepancy.

Flagged Code
bool VerifyToken(string token, byte[] key, byte[] payload) {
   using var hmac = new HMACSHA256(key);
   string expected = Convert.ToHexString(hmac.ComputeHash(payload));
   return expected == token;   // flagged: == returns as soon as a byte differs
}
Fixed Code
bool VerifyToken(string token, byte[] key, byte[] payload) {
   using var hmac = new HMACSHA256(key);
   byte[] expected = hmac.ComputeHash(payload);
   byte[] provided = Convert.FromHexString(token);
   return CryptographicOperations.FixedTimeEquals(expected, provided);
}

How to fix

Compare secrets with System.Security.Cryptography.CryptographicOperations.FixedTimeEquals(a, b) on the binary form of both values. For two strings, compare their UTF-16 bytes — MemoryMarshal.AsBytes(a.AsSpan()) vs MemoryMarshal.AsBytes(b.AsSpan()) — which is the exact ordinal equality. Do NOT re-encode them to UTF-8 first: an unpaired surrogate becomes U+FFFD, so two different strings can encode to the same bytes and compare equal. For passwords, compare the stored hash with the freshly computed hash the same way. Suppress the issue when the compared values are not secrets.

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>Review secrets compared with non-constant-time equality</Name>
// <Id>ND3113:ReviewSecretsComparedWithNonConstantTimeEquality</Id>
warnif count > 0

// Third-party members are part of the code model only when the application uses them.
let equalityMembers = ThirdParty.Types.WithFullNameIn(
  "System.String", "System.StringComparer", "System.Linq.Enumerable", "System.MemoryExtensions")
  .ChildMethods().Where(m => m.SimpleNameEqualsAny("Equals", "op_Equality", "op_Inequality", "Compare", "CompareOrdinal", "SequenceEqual"))
  .ToHashSetEx()

// Computing a MAC, hash or derived key and then comparing it is the strongest signal.
let hashComputingMembers = ThirdParty.Methods
  .Where(m => m.ParentNamespace.Name == "System.Security.Cryptography" &&
              m.SimpleNameEqualsAny("ComputeHash", "TryComputeHash", "HashData", "TryHashData", "GetBytes", "Pbkdf2", "GetHashAndReset"))
  .ToHashSetEx()

let constantTimeEquals = ThirdParty.Types.WithFullName("System.Security.Cryptography.CryptographicOperations")
  .ChildMethods().Where(m => m.SimpleName == "FixedTimeEquals").ToHashSetEx()

where equalityMembers.Any()

from m in Application.Methods.UsingAny(equalityMembers)
where !m.MethodsCalled.Intersect(constantTimeEquals).Any()
let comparesComputedHash = m.MethodsCalled.Intersect(hashComputingMembers).Any()
// A name like VerifyToken(), ValidateSignature(), IsPasswordValid(), RedeemPasswordResetPin() or CheckApiKey()
let nameSuggestsSecret =
   m.SimpleName.ContainsAny(new[] { "Token", "Signature", "Hmac", "Secret", "Otp", "Password", "Passwd", "ApiKey", "Digest", "Nonce", "Csrf", "Antiforgery" }, StringComparison.OrdinalIgnoreCase) &&
   m.SimpleName.ContainsAny(new[] { "Verify", "Validate", "Valid", "Check", "Compare", "Match", "Authenticate", "Redeem", "Confirm", "Equal" }, StringComparison.OrdinalIgnoreCase) &&
  !m.SimpleName.ContainsAny(new[] { "Cancellation", "Tokenize", "Tokenizer", "PublicKeyToken", "HelpToken" }, StringComparison.OrdinalIgnoreCase)
where comparesComputedHash || nameSuggestsSecret
select new {
  m,
  equalityMembersUsed = m.MethodsCalled.Intersect(equalityMembers).ToArray(),
  comparesComputedHash,
  Debt = 10.ToMinutes().ToDebt(),
  Severity = comparesComputedHash ? Severity.High : Severity.Medium
}

//<Expl>
// The *{0}* {2?computes a hash or MAC and compares it:has a name suggesting it checks a secret}
// with *{1}*, which returns as soon as the first byte differs. An attacker
// measuring response times can recover the expected value byte by byte.
//</Expl>

//<Desc>
// Matches methods that compare strings or byte sequences with
// *String.Equals()*, *==*, *String.Compare()* or *SequenceEqual()* and
// either compute a hash / MAC / derived key in the same body, or carry a
// name mentioning a token, signature, HMAC, secret, OTP, password, API key,
// digest or nonce. Ordinary equality short-circuits on the first mismatch,
// so the elapsed time leaks how many leading bytes were right: a timing
// side channel that has been exploited over networks.
//
// This is CWE-208 *Observable Timing Discrepancy*.
//
// **Flagged Code**
// ```csharp
// bool VerifyToken(string token, byte[] key, byte[] payload) {
//    using var hmac = new HMACSHA256(key);
//    string expected = Convert.ToHexString(hmac.ComputeHash(payload));
//    return expected == token;   // flagged: == returns as soon as a byte differs
// }
// ```
//
// **Fixed Code**
// ```csharp
// bool VerifyToken(string token, byte[] key, byte[] payload) {
//    using var hmac = new HMACSHA256(key);
//    byte[] expected = hmac.ComputeHash(payload);
//    byte[] provided = Convert.FromHexString(token);
//    return CryptographicOperations.FixedTimeEquals(expected, provided);
// }
// ```
//</Desc>

//<HowToFix>
// Compare secrets with
// *System.Security.Cryptography.CryptographicOperations.FixedTimeEquals(a, b)*
// on the binary form of both values. For two strings, compare their UTF-16
// bytes — *MemoryMarshal.AsBytes(a.AsSpan())* vs *MemoryMarshal.AsBytes(b.AsSpan())*
// — which is the exact ordinal equality. Do NOT re-encode them to UTF-8 first:
// an unpaired surrogate becomes U+FFFD, so two different strings can encode
// to the same bytes and compare equal. For passwords, compare the stored hash
// with the freshly computed hash the same way. Suppress the issue when the
// compared values are not secrets.
//</HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items