alexedwards / argon2id

Argon2id password hashing and verification for Go
MIT License
452 stars 44 forks source link

Password matches with invalid hashes #11

Closed seremenko-wish closed 3 years ago

seremenko-wish commented 3 years ago

Test for reproduction

func TestBugReproduction(t *testing.T) {
    // "bug" valid hash: $argon2id$v=19$m=65536,t=1,p=2$UDk0zEuIzbt0x3bwkf8Bgw$ihSfHWUJpTgDvNWiojrgcN4E0pJdUVmqCEdRZesx9tE
    ok, _, err := CheckHash("bug", "$argon2id$v=19$m=65536,t=1,p=2$UDk0zEuIzbt0x3bwkf8Bgw$ihSfHWUJpTgDvNWiojrgcN4E0pJdUVmqCEdRZesx9tE")
    if err != nil {
        t.Fatal(err)
    }
    if !ok {
        t.Fatal("expected password to match")
    }

    // changed one last character of the hash
    ok, _, err = CheckHash("bug", "$argon2id$v=19$m=65536,t=1,p=2$UDk0zEuIzbt0x3bwkf8Bgw$ihSfHWUJpTgDvNWiojrgcN4E0pJdUVmqCEdRZesx9tF")
    if err == nil {
        t.Fatal("Hash validation should fail")
    }

    if ok {
        t.Fatal("Hash validation should fail")
    }

       // same can be done with salt
}

The issue is in base64 decrypt function call here https://github.com/alexedwards/argon2id/blob/master/argon2id.go#L165 and here https://github.com/alexedwards/argon2id/blob/master/argon2id.go#L171. If you don't use strict mode for these calls, you end up successfully matching invalid hashes.

More info:

Solution:

alexedwards commented 3 years ago

Thanks so much for the clear report and failing test. I've just pushed commit https://github.com/alexedwards/argon2id/commit/e2135f7c9c7702071aa8c0881f593a3f039e69da which fixes this.

seremenko-wish commented 3 years ago

Thanks for fixing it in the article too :) It is in the first position for many google searches related to using Argon2 with Golang, and obviously that this issue leaked to many projects

TorchedSammy commented 3 years ago

since this is fixed can this issue be closed?