Skip to content

CWE-597: Use of Wrong Operator in String Comparison - C#

Overview

C# is not Java here, and the difference decides how to triage the finding. When both operands have the static type string, == calls String.Equals and performs an ordinal value comparison - not reference equality, and not something interning can change. Measured on .NET 10, a string built at runtime with a StringBuilder compares equal to the literal with == while ReferenceEquals on the same pair is false. So a scanner flagging password == storedPassword on two string variables has not found the Java bug, and rewriting it as .Equals() changes nothing about what the program does.

Four things in C# genuinely do go wrong, and a CWE-597 finding is worth reading against them in this order:

  • The static type is not string. An object, or a generic T constrained to a reference type, selects the reference-equality operator, so the comparison is against identity. The compiler warns for someObject == "literal" (CS0252) and says nothing at all when both operands are object or a generic parameter.
  • A case fold happens before the comparison. ToLower() and ToUpper() with no argument use the current thread's culture, which under ASP.NET Core request localization is chosen by the caller's Accept-Language header.
  • The comparison is a prefix or a search, not an equality test. StartsWith, EndsWith, IndexOf(string), CompareTo and String.Compare default to CurrentCulture where the equality APIs default to ordinal, so the trap is in the neighbouring method rather than in Equals.
  • The value is a secret. == compares it correctly and not in constant time, which is a different weakness (CWE-208) reported against the same line.

Primary Defence: Keep security-relevant comparisons on operands whose static type is string, use StringComparison.Ordinal or OrdinalIgnoreCase explicitly on every prefix, search and case-insensitive comparison, never fold case with a culture-dependent ToLower()/ToUpper(), and use CryptographicOperations.FixedTimeEquals() for secrets. Prefer an enum over a string wherever the set of values is fixed.

Common Vulnerable Patterns

Reference Equality Through an object or Generic Parameter

// VULNERABLE - the static type is object, so == is reference equality
[Authorize]
public IActionResult AdminPanel()
{
    // A generic settings/claims/reflection API that returns object
    object role = _claims.GetValue("role");   // holds the string "ADMIN"

    if (role == "ADMIN")  // reference comparison - never true for a parsed string
    {
        return View("Admin");
    }
    return Forbid();
}

// The same bug with no compiler diagnostic at all:
static bool Matches<T>(T a, T b) where T : class => a == b;   // always reference equality

Why this is vulnerable: == is resolved at compile time from the static types of its operands. Two string operands select String.op_Equality and compare content; the moment one side is object, dynamic's static counterpart, or an unconstrained-to-string generic T, the compiler selects object.ReferenceEquals instead and the comparison asks whether the two are the same instance. A role, claim or header value that was parsed out of a request or read from a database is a fresh instance, so the test is false whatever it contains. Measured on .NET 10: object == "ADMIN" against a runtime-built "ADMIN" returns false, and so does Matches<string>(built, "ADMIN"). The diagnostic is inconsistent and that is what lets it survive - the compiler emits CS0252 for object == "literal", and emits nothing when both operands are object or when the comparison sits inside a generic method, which is where refactoring usually puts it.

A Reference Comparison Inside a Denylist

// VULNERABLE - a denylist that can never match grants instead of denying
static class Compare
{
    // Shared "is this one of these?" helper. T is a reference type, so == here
    // is reference equality no matter what the caller passes.
    public static bool IsAnyOf<T>(T value, params T[] candidates) where T : class
    {
        foreach (T candidate in candidates)
        {
            if (value == candidate) return true;
        }
        return false;
    }
}

[HttpPost("register")]
public IActionResult Register(RegistrationRequest request)
{
    if (Compare.IsAnyOf(request.Username, "admin", "root", "system"))
    {
        return BadRequest("That username is reserved.");
    }

    _users.Create(request.Username);   // "admin" is created
    return Ok();
}

Why this is vulnerable: This is the same mistake as the pattern above, in the one direction where it is a bypass rather than an outage, and in the one place where the compiler is silent about it. A broken positive check (if (role == "ADMIN") allow) denies everybody, including whoever tests it, so it rarely survives to production. A broken negative check fails the other way: the branch that would have rejected the request is never taken, so the endpoint answers 200 and every test that asserts a normal registration succeeds still passes. Nothing reports a problem, because as far as the program is concerned the name was not on the list. Measured on .NET 10, IsAnyOf(username, "admin", ...) returns false for a username deserialized from the request body and the account is created; the same comparison written inline as username == reservedObject would at least have produced CS0253, but moving it into a shared generic helper - which is what a tidy-up commit does - removes the diagnostic while keeping the bug. Any comparison whose false branch is the permissive one deserves this reading: reserved names, blocked extensions, denied paths, revoked key IDs.

Culture-Sensitive Case Folding Before the Comparison

// VULNERABLE - ToLower() with no culture; the caller may choose the culture
[HttpGet("resource")]
public IActionResult GetResource(string name)
{
    if (name.ToLower() == "admin")          // fold happens in CurrentCulture
    {
        return Forbid();                    // protected name
    }

    return Ok(_resources.Get(name));
}
# Program.cs enables request localization with Turkish among the supported cultures
GET /resource?name=ADMIN                          -> 403  (culture en-US, folds to "admin")
GET /resource?name=ADMIN
Accept-Language: tr-TR                            -> 200  (culture tr-TR, folds to "admin" with a dotless i)

Why this is vulnerable: ToLower() and ToUpper() without a CultureInfo argument use CultureInfo.CurrentCulture. In Turkish and Azeri, I folds to ı (U+0131, dotless i) rather than to i, so "ADMIN".ToLower() is "admın" and no longer equals the literal the check compares against. Measured on .NET 10 against a minimal ASP.NET Core app with UseRequestLocalization and tr-TR among the supported cultures: the request above returned 403 with no header and 200 with Accept-Language: tr-TR, serving the protected name. Two qualifications worth carrying into triage, because they decide whether the finding is remotely reachable or only environment-dependent. The header only moves the culture when the localization middleware is installed and the requested culture is in SupportedCultures - with the middleware removed, the same request stayed on the server's culture and was refused. And the payload has to contain a letter whose fold is locale-dependent: "DELETE".ToLower() is "delete" in every culture, so a check on that word is unaffected. Where neither holds, the weakness is still real but its trigger is the server's own locale, which changes on redeployment rather than on request.

Culture-Sensitive Prefix and Search APIs

// VULNERABLE - StartsWith, IndexOf and CompareTo default to CurrentCulture
public bool HasBearerScheme(string authorizationHeader)
{
    return authorizationHeader.StartsWith("Bearer ");   // culture-sensitive overload
}

public bool BelongsToTenant(string resourceKey, string tenantPrefix)
{
    return resourceKey.IndexOf(tenantPrefix) == 0;      // culture-sensitive overload
}

Why this is vulnerable: The equality APIs and the search APIs disagree about their default, and only the equality half is ordinal. ==, string.Equals(a, b) and a.Equals(b) compare ordinally; StartsWith(string), EndsWith(string), IndexOf(string), CompareTo and String.Compare compare using CurrentCulture. That produces two separate problems, and only one of them is locale-dependent.

The first is that ICU collation treats format characters as ignorable rather than as data, so a padded value satisfies a prefix test it does not literally satisfy. Measured on .NET 10 with SH standing for U+00AD (soft hyphen): "Bea<SH>rer token-value".StartsWith("Bearer ") is true with the default overload and false with StringComparison.Ordinal, and so is the leading-SH variant; "tenant<SH>-42/report.pdf".IndexOf("tenant-42/") == 0 is likewise true under culture and false under ordinal. So BelongsToTenant accepts a key that does not begin with the tenant's prefix, and whatever reads the key afterwards - a storage path, a database filter - will use the literal bytes rather than the collated reading. This behaviour is not locale-specific: all three results were identical under en-US, tr-TR and da-DK.

The second is ordering, which genuinely does vary by locale. Measured on the same runtime, "aa".CompareTo("ab") returns -1 under en-US and tr-TR and +1 under da-DK, because Danish collates aa as \u00E5 and sorts it after z; "AA".CompareTo("aa") flips from +1 to -1 across the same pair. Any range check, sort-order assumption or Compare(...) == 0 test built on the default overload changes answer when the process locale changes, which is the "works on one server, fails on another" behaviour CWE-597 describes.

The two overloads do not fail in the same direction, which is why "switch to ordinal" needs stating carefully. Ordinal is the right default because it is predictable, not because it is stricter: here the culture overload matched more than the bytes justify, but for a denylist that same property is what catches padded input, and ordinal is the one that would let it past. Normalize input before a denylist rather than assuming either overload is the safe one - see the Considerations section.

Secure Patterns

Keep the Static Type string, and Say Ordinal Where It Matters

// SECURE - string-typed operands; == and Equals are ordinal by definition
[Authorize]
public IActionResult AdminPanel()
{
    string? role = _claims.GetValue("role") as string;   // cast once, at the boundary

    if (string.Equals(role, "ADMIN", StringComparison.Ordinal))
    {
        return View("Admin");
    }
    return Forbid();
}

// Prefix and search comparisons need the argument spelled out - they do not
// inherit the equality APIs' ordinal default.
bool blocked = url.StartsWith("javascript:", StringComparison.OrdinalIgnoreCase);
int  index   = path.IndexOf("..", StringComparison.Ordinal);

Why this works: Casting to string at the point the value leaves a weakly typed API restores the value-equality operator for every comparison downstream, and does it in one place rather than at each call site. string.Equals(a, b, StringComparison.Ordinal) is then belt and braces: it is what == already does, but it survives a later refactor that widens the variable's type, because the static overload takes two string parameters and a widening change becomes a compile error instead of a silent behaviour change. Naming StringComparison on StartsWith and IndexOf is not redundant in the same way - there it changes the behaviour, from ICU collation to a code-unit comparison that gives the same answer on every machine and every locale. Analyzer rule CA1307 flags the culture-dependent overloads and is worth turning on as an error.

Case-Insensitive Comparison Without the Culture

// SECURE - one call, no intermediate string, no culture
if (string.Equals(name, "admin", StringComparison.OrdinalIgnoreCase))
{
    return Forbid();
}

// If the value must be folded rather than compared - a lookup key, a cache key -
// name the culture explicitly:
string key = name.ToLowerInvariant();          // not name.ToLower()

Why this works: StringComparison.OrdinalIgnoreCase performs a simple, locale-independent case fold as part of the comparison, so "ADMIN", "Admin" and "admin" all match and the Turkish dotless-i behaviour never enters. It also allocates nothing, where ToLower() builds a second string on every call. Where a fold genuinely has to happen - normalizing a key before storing it, not comparing it - ToLowerInvariant() and ToUpperInvariant() are the culture-fixed forms; ToUpperInvariant() is the one to prefer for a value that will later be compared, because it round-trips a few characters that lowercasing does not.

Enum-Based Authorization

// SECURE - use enums for roles
public enum Role
{
    User,
    Admin,
    Moderator
}

[Authorize]
[HttpGet("admin")]
public IActionResult AdminPanel()
{
    Role userRole = _userService.GetUserRole(User.Identity?.Name);

    // Safe to use == with enums (value types)
    if (userRole == Role.Admin)
    {
        return View("Admin");
    }

    return Forbid();
}

// Entity Framework Core model
public class User
{
    public int Id { get; set; }
    public string Username { get; set; } = "";
    public Role Role { get; set; }
}

// EF Core maps an enum to its underlying integer by default. Ask for the string
// column explicitly if you want the database to be readable:
protected override void OnModelCreating(ModelBuilder b) =>
    b.Entity<User>().Property(u => u.Role).HasConversion<string>();

Why this works: Enums are value types, so == compares their underlying integral values and there is no reference-equality trap to fall into. The comparison cannot be widened to object by accident either: object boxed = Role.Admin; boxed == Role.Admin is CS0019, a compile error rather than the silent behaviour change the same refactor causes on a string. Using an enum also moves the set of valid roles into the type system: a typo like Role.Admn is a compile error, where "ADMN" is a check that silently never matches - which, in a denylist, is the bypass in the second vulnerable pattern above. The mapping detail is worth getting right rather than assuming: measured on EF Core 10, a plain Role property generates "Role" INTEGER NOT NULL, so the HasConversion<string>() call is what produces a string column, not the default.

Constant-Time Comparison for Secrets

// SECURE - constant-time comparison (.NET Core 2.1+)
using System.Security.Cryptography;
using System.Text;

[HttpPost("api/validate")]
public IActionResult ValidateToken([FromHeader(Name = "X-Api-Token")] string? token)
{
    var sessionToken = HttpContext.Session.GetString("apiToken");

    if (sessionToken is null || token is null)
    {
        return Unauthorized();
    }

    // Hash both sides so the comparison always walks 32 bytes, whatever was
    // submitted - FixedTimeEquals returns false on a length difference, which
    // would otherwise make the token's length measurable.
    byte[] expected = SHA256.HashData(Encoding.UTF8.GetBytes(sessionToken));
    byte[] provided = SHA256.HashData(Encoding.UTF8.GetBytes(token));

    if (CryptographicOperations.FixedTimeEquals(expected, provided))
    {
        return Ok("Valid token");
    }

    return Unauthorized();
}

Why this works: CryptographicOperations.FixedTimeEquals() compares two spans without an early exit, so the time it takes does not depend on how many leading bytes matched. It returns false for a length mismatch rather than throwing, so a truncated attacker token produces a 401 and not a 500 - unlike Node's timingSafeEqual, which raises. Hashing both operands first fixes the comparison at 32 bytes so that the length difference never reaches the call, and it costs one SHA-256 over a short string. The explicit null checks are what keep the endpoint answering 401 for a missing header instead of crashing; [FromHeader] binds a missing header to null without complaint.

Null-Safe Comparison

// SECURE - reject the absent value, then compare what is left
public bool MatchesUsername(User? user, string? targetUsername)
{
    if (user?.Username is null || targetUsername is null)
    {
        return false;   // "no username" is not a match for "no username"
    }

    return string.Equals(user.Username, targetUsername, StringComparison.Ordinal);
}

// string.Equals static method behavior - note the first line:
//   string.Equals(null, null, StringComparison.Ordinal) => true
//   string.Equals("test", null, StringComparison.Ordinal) => false
//   string.Equals(null, "test", StringComparison.Ordinal) => false

Why this works: The static string.Equals(string?, string?, StringComparison) overload is a plain method call rather than a dereference, so a null on either side is an ordinary argument value and not a NullReferenceException. That is what makes it safe to reach for where the instance method would crash - but it is not on its own a null check, and the explicit guard above is doing real work rather than being defensive noise. Measured on .NET 10, dropping it and returning string.Equals(user?.Username, targetUsername, StringComparison.Ordinal) directly returns true for a null user and a null targetUsername, and true again for a User whose Username is null compared against a null target: ?. turns the missing navigation into null, and two nulls compare equal.

For an identity check that is the wrong answer in the most dangerous direction - an unauthenticated caller with no username supplied matches a record with no username stored. Anywhere the comparison decides who someone is, "absent" must not equal "absent": reject it first, as above, and let the ordinal comparison see only two real strings. Where a null genuinely should match a null - reconciling two records, diffing a form against stored state - the bare string.Equals call is right, and saying which of the two you meant is the point.

CSRF Token Validation Best Practice

// SECURE - proper CSRF token validation
using Microsoft.AspNetCore.Antiforgery;

public class SecureController : Controller
{
    private readonly IAntiforgery _antiforgery;

    public SecureController(IAntiforgery antiforgery)
    {
        _antiforgery = antiforgery;
    }

    [HttpPost("transfer")]
    [ValidateAntiForgeryToken]  // Best: Use built-in attribute
    public IActionResult Transfer(TransferRequest request)
    {
        ProcessTransfer(request);
        return Ok("Transfer complete");
    }

    // Manual validation (if needed):
    [HttpPost("custom")]
    public async Task<IActionResult> CustomEndpoint()
    {
        // Throws AntiforgeryValidationException when the token is missing or wrong
        await _antiforgery.ValidateRequestAsync(HttpContext);

        ProcessRequest();
        return Ok();
    }
}

Why this works: ASP.NET Core's antiforgery system generates cryptographically random tokens, protects them with the data-protection stack, and validates them itself, so there is no application-level string comparison to get wrong - no operator choice, no culture, no timing question. [ValidateAntiForgeryToken] short-circuits the request before the action runs. ValidateRequestAsync is the manual equivalent and throws AntiforgeryValidationException rather than returning a boolean, so it must either be left to propagate to a handler that turns it into a 400, or caught explicitly; awaiting it and ignoring the result is not a validation.

Considerations

  • Decide first whether the finding is the Java bug or not. For two string-typed operands it is not: == is already an ordinal value comparison, and changing it to .Equals() is a no-op. Recording that as a false positive with the static types written down is a legitimate outcome and a faster one than a rewrite that changes nothing. What is worth a change is the type on either side of the operator - if a value arrives from a claims bag, a configuration binder, a reflection call or a generic helper, cast it to string at that boundary so the operator cannot be reselected later.
  • Whether a culture-sensitive fold is reachable depends on two things you can check quickly. Is UseRequestLocalization in the pipeline, and is a culture with different casing rules in SupportedCultures? If both are true, the caller picks the culture and the finding is remotely triggerable. If not, the culture is whichever locale the process starts in, which makes the bug real but environment-dependent - it surfaces on a redeploy to a differently configured host rather than on a request. Either way the fix is the same one line; the answer only changes how urgent it is.
  • A secret comparison on this page is usually two other findings wearing a CWE-597 label. csrf.Equals(sessionToken) on two string operands is an ordinal comparison, so there is no operator bug to fix - but there are two real defects on that line and neither is this CWE. It is not constant-time, so the call's duration reflects how much of the submitted token matched (CWE-208 has what that does and does not leak on .NET, and the Constant-Time Comparison for Secrets pattern above has the replacement). And the instance form dereferences its receiver, so a bound string parameter for a field the request omitted arrives as null and the endpoint answers 500 instead of 403 - a crash on the malicious input, which passes every test that only checks a forged token is refused, and which is distinguishable from a genuine refusal. The static string.Equals(a, b) takes nulls on both sides; the instance method does not. Re-file rather than "fixing" the operator.
  • The comparison and its neighbours have different defaults, so check what else the method does with the string. A finding on Equals is often a symptom of a file where StartsWith and IndexOf are also written with the default overload. Enable CA1307 (Specify StringComparison for clarity) as an error and let it enumerate them, rather than fixing the one line the scanner named.
  • Ordinal is predictable, not automatically stricter. For an allowlist that is what you want, and it is what the tenant-prefix measurement above argues for. For a denylist the property inverts: culture comparison ignores exactly the characters an attacker would pad with, so moving that check to ordinal can widen what gets through. Reject or strip the padding before the check rather than relying on either overload, and check what your chosen normalization actually does - measured on .NET 10, string.Normalize(NormalizationForm.FormKC) leaves both U+00AD and U+200D in place, so it is not the control it reads as. Filtering on CharUnicodeInfo.GetUnicodeCategory(c) != UnicodeCategory.Format does remove them.

Testing

A rule that finds == on strings cannot distinguish the case that matters from the case that does not, and a rule that finds ToLower() cannot tell whether the culture is reachable. These are what to assert instead:

  • The accept, not only the reject. Call the corrected comparison with a value built at runtime - new StringBuilder(...), a value round-tripped through JsonSerializer, a value read back from the database - rather than with a literal, and assert it matches. A comparison that is still against identity passes every rejection test and fails only this one.
  • A denylist entry, through the endpoint. Post the reserved value and assert the specific rejection - 400 with the reserved-name message, not just "not 200". A 200 here is the second vulnerable pattern above.
  • The same request under Accept-Language: tr-TR. Assert the outcome is identical to the request without the header. If they differ, the fold is still culture-dependent and the caller is choosing it. Run the same assertion after setting CultureInfo.DefaultThreadCurrentCulture = new CultureInfo("tr-TR") in test setup, which covers the deployment-locale case even where the middleware is absent.
  • A missing value, not a wrong one. Send the request with the token, header or field omitted entirely and assert 401/403 with a JSON body - a 500 means something dereferenced a null and the error path is distinguishable from a genuine refusal.
  • The comparison narrowed something. Where the check gates a query rather than a branch, assert the result set differs between an authorized and an unauthorized caller. A comparison that never matches returns the unfiltered set while every negative test still passes.

Additional Resources