When running our Web Security Code Review Training, I use an analogy on the difference between "They are French" and "They speak French". People tend to mix both sentences as if they meant the exact same thing. They don’t. And that is a key principle of security code review (or any code review really). Basically, you want to find places where developers meant one thing but wrote another. Once you find some of those, it’s time to investigate.

Such example can be found in the following code from ASP.NET Core:

    public virtual async Task<bool> CheckPasswordAsync(TUser user, string password)
    {
        ThrowIfDisposed();
        var passwordStore = GetPasswordStore();
        if (user == null)
        {
            return false;
        }
        
        var result = await VerifyPasswordAsync(passwordStore, user, password).ConfigureAwait(false);
        if (result == PasswordVerificationResult.SuccessRehashNeeded)
        {
            await UpdatePasswordHash(passwordStore, user, password, validatePassword: false).ConfigureAwait(false);
            await UpdateUserAsync(user).ConfigureAwait(false);
        }
            
        var success = result != PasswordVerificationResult.Failed;
        if (!success)
        {
            Logger.LogDebug(LoggerEventIds.InvalidPassword, "Invalid password for user.");
        }
        return success;
    }

The line:

        var success = result != PasswordVerificationResult.Failed;

is used to decide if the authentication was succesful or not. This code does not check if the PasswordVerificationResult was successful, it checks that the result was not PasswordVerificationResult.Failed. There is a very small distinction and there is room for mistakes. If we can find a way to return something that is not PasswordVerificationResult.Failed, we may be into something...

What we want now is to find some code that will return something along the line of PasswordVerificationResult.SomethingIsNotRight, allowing us to bypass the check:

        var success = result != PasswordVerificationResult.Failed;

What if we provide an empty password, what if the password in the database is corrupted, what if the computation of the hash fails…

Now, as a security code reviewer, it is time to check all the code paths and try to find one that gets the code to return something that is not PasswordVerificationResult.Failed. If we find one, we potentially have a way to bypass the authentication mechanism.

A second option is to look at the declaration of PasswordVerificationResult:

public enum PasswordVerificationResult
{
    /// 

    /// Indicates password verification failed.
    /// 
    Failed = 0,

    /// 

    /// Indicates password verification was successful.
    /// 
    Success = 1,

    /// 

    /// Indicates password verification was successful however the password was encoded using a deprecated algorithm
    /// and should be rehashed and updated.
    /// 
    SuccessRehashNeeded = 2
}

We can see that it is an enum that can only take three values. This tells us that we only have two options: Failed and Success (since SuccessRehashNeeded is just a variant of Success). Unfortunately we don’t get to bypass the authentication.

While this specific investigation didn’t lead to an authentication bypass in ASP.NET Core, it illustrates a key element of Web Security Code Review: how small discrepancies between what developers want to say and what developers wrote can lead to security issues.