Hi,

On IRC, someone and I were arbitrarily going through bcrypt.c and I noticed
the following resulting from bcrypt_newhash():

int
bcrypt_newhash(const char *pass, int log_rounds, char *hash, size_t hashlen)
{
        char salt[BCRYPT_SALTSPACE];

        if (bcrypt_initsalt(log_rounds, salt, sizeof(salt)) != 0)
                return -1;
...

Here at the end salt is later explicit_bzero'd.  but inside the 
bcrypt_initsalt() function there is also an internal csalt array.

It's on the stack so it's likely overwritten, so I'm wondering if this is even
right to explicity_bzero csalt? 

I haven't tested this patch before I get someone to judge whether it's right
or wrong?

Patch follows after my sig,

Best Regards,
-peter


Index: bcrypt.c
===================================================================
RCS file: /cvs/src/lib/libc/crypt/bcrypt.c,v
retrieving revision 1.58
diff -u -p -u -r1.58 bcrypt.c
--- bcrypt.c    6 Jul 2020 13:33:05 -0000       1.58
+++ bcrypt.c    21 Jan 2021 14:26:10 -0000
@@ -82,6 +82,8 @@ bcrypt_initsalt(int log_rounds, uint8_t 
        snprintf(salt, saltbuflen, "$2b$%2.2u$", log_rounds);
        encode_base64(salt + 7, csalt, sizeof(csalt));
 
+       explicit_bzero(csalt, sizeof(csalt));
+
        return 0;
 }
 


Reply via email to