P-H-C / phc-winner-argon2

The password hash Argon2, winner of PHC
Other
4.78k stars 406 forks source link

Problem with verify passord - returns OK if string is different #303

Closed nicraMarcin closed 4 years ago

nicraMarcin commented 4 years ago

Hello, I have strange problem with verify password. I generated argon2 string with password password. Now when I verify it I got OK if password isnt the same but beginning of string matches to password. For example. encoded password = password verification:

password - OK
passwort - NOK
passworddddd - OK
fakepass - NOK
password12234  - OK

I have notice, that only 8 first characters are chcecked in password. I genereated verylongpasswordfortest and when I input verylong text password is still ok, but when I input 7 characters password is incorrect

my code: method for encoding password

oatpp::String nms::AuthService::encodePassword(oatpp::String &plainPassword) {
    const char *salt = nms::helper::gen_salt().c_str(); // <-- returns 8 characters
    char encoded[97] = {};

    argon2id_hash_encoded(2, 1<<16, 2, plainPassword->c_str(), sizeof(plainPassword->c_str()), salt, sizeof(salt), 32, encoded, std::size(encoded));

    return oatpp::String(encoded);
}

method for verify password

bool nms::AuthService::verifyPassword(const oatpp::String &username, const oatpp::String &plainPassword) {

    OATPP_LOGD("AuthService::verifyPassword", "username: %s, pwd: %s", username->c_str(), plainPassword->c_str())

    const char *pwd = plainPassword->c_str(); // <-- this is const *
    const char *argon2string = "$argon2id$v=19$m=65536,t=2,p=2$SnkwZmJEQ0I$A9VmIhn+zfdahHfruPCIQHwnJvJjA4bHhcwcPNkIb3w";
    int validityCode = argon2id_verify(argon2string, pwd, sizeof(pwd)); // <-- 'password' string

    OATPP_LOGD("AuthService::verifyPassword", "Code: %s", argon2_error_message(validityCode))
    if (validityCode == 0 ) return true;

    return false;
}

Now I test it an in my log:

DEBUG (AuthService::verifyPassword): username: admin, pwd: password
DEBUG (AuthService::verifyPassword): Code: OK
DEBUG (AuthService::verifyPassword): username: admin, pwd: fakepasss
DEBUG (AuthService::verifyPassword): Code: The password does not match the supplied hash
DEBUG (AuthService::verifyPassword): username: admin, pwd: passwordddd
DEBUG (AuthService::verifyPassword): Code: OK
DEBUG (AuthService::verifyPassword): username: admin, pwd: password1233whatether
DEBUG (AuthService::verifyPassword): Code: OK

What I'm doing wrong?

argon2 tests

$ make test
Building with optimizations for native
cc -std=c89 -O3 -Wall -g -Iinclude -Isrc -pthread -march=native  -Wextra -Wno-type-limits src/argon2.c src/core.c src/blake2/blake2b.c src/thread.c src/encoding.c src/opt.c src/test.c -o testcase
Default build
argon2i v=16: OK
argon2d v=16: OK
argon2id v=16: OK
argon2i v=19: OK
argon2d v=19: OK
argon2id v=19: OK
Force OPTTEST=1
argon2i v=16: OK
argon2d v=16: OK
argon2id v=16: OK
argon2i v=19: OK
argon2d v=19: OK
argon2id v=19: OK
./testcase
Test Argon2i version number: 10
Hash test: $v=16 t=2, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=16 t=2, m=18, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=16 t=2, m=8, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=16 t=2, m=8, p=2, pass=password, salt=somesalt: PASS
Hash test: $v=16 t=1, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=16 t=4, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=16 t=2, m=16, p=1, pass=differentpassword, salt=somesalt: PASS
Hash test: $v=16 t=2, m=16, p=1, pass=password, salt=diffsalt: PASS
Recognise an invalid encoding: PASS
Recognise an invalid encoding: PASS
Recognise an invalid salt in encoding: PASS
Verify with mismatched password: PASS
Decode an error message: PASS

Test Argon2i version number: 13
Hash test: $v=19 t=2, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=18, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=8, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=8, p=2, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=1, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=4, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=16, p=1, pass=differentpassword, salt=somesalt: PASS
Hash test: $v=19 t=2, m=16, p=1, pass=password, salt=diffsalt: PASS
Recognise an invalid encoding: PASS
Recognise an invalid encoding: PASS
Recognise an invalid salt in encoding: PASS
Verify with mismatched password: PASS
Decode an error message: PASS

Test Argon2id version number: 13
Hash test: $v=19 t=2, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=18, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=8, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=8, p=2, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=1, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=4, m=16, p=1, pass=password, salt=somesalt: PASS
Hash test: $v=19 t=2, m=16, p=1, pass=differentpassword, salt=somesalt: PASS
Hash test: $v=19 t=2, m=16, p=1, pass=password, salt=diffsalt: PASS

Common error state tests
Fail on invalid memory: PASS
Fail on invalid null pointer: PASS
Fail on salt too short: PASS
sneves commented 4 years ago

int validityCode = argon2id_verify(argon2string, pwd, sizeof(pwd)); // <-- 'password' string

sizeof(pwd) is the size of a pointer, i.e., typically 8 bytes. You're effectively only checking the first 8 bytes of passwords.

The fix would look like int validityCode = argon2id_verify(argon2string, pwd, plainPassword->size());

nicraMarcin commented 4 years ago

@sneves Yes, correct, this is pointer :) :+1: I have also generated hash from 8 first characters :) argon2id_hash_encoded(2, 1<<16, 2, plainPassword->c_str(), sizeof(plainPassword->c_str()), salt, sizeof(salt), 32, encoded, std::size(encoded));