alexedwards / argon2id

Argon2id password hashing and verification for Go
MIT License
467 stars 45 forks source link

fix security flaw in generateRandomBytes #14

Closed s0rg closed 2 years ago

s0rg commented 2 years ago

You need to use io.ReadFull instead of rand.Read. Please see this issue: https://github.com/satori/go.uuid/issues/73

alexedwards commented 2 years ago

Please double-check my understanding of this, but as far as I can see this package uses the crypto/rand package from the standard library and calls rand.Read() to generate the random bytes.

The documentation for rand.Read() says:

Read is a helper function that calls Reader.Read using io.ReadFull. On return, n == len(b) if and only if err == nil.

Looking at the source code for rand.Read() confirms that it uses io.ReadFull behind the scenes. See https://cs.opensource.google/go/go/+/refs/tags/go1.19:src/crypto/rand/rand.go;l=24.

In the issue you linked to in the description, it looks like rand did not reference the crypto/rand package, but rather a struct field with the type io.Reader, which is what led to the problem for that package.

As far as I can see, that's not the case here, and a full read is already ensured by using rand.Read() from crypto/rand.

s0rg commented 2 years ago

I missed this fix in crypto/rand thank you.

Youre code is correct )