I guess so, though I cannot imagine a program-context where bcrypt_newhash
stack's can be leaked in a dangerous way.

Programs that create these hashes end up knowing more, or having more power.

Peter J. Philipp <[email protected]> wrote:

> 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