Re: Potential int overflow in src/sbin/bioctl/pbkdf2.c

[Mike Belopuhov]

On Thu, Jun 28, 2012 at 16:04 +0100, Andrew Nelless wrote:
> Hi,
> 
> The range checking of the salt length (salt_len) in pkcs5_pbkdf2() on
> line 90 of  src/sbin/bioctl/pbkdf2.c is a bit off:
> 
>   90: if (salt_len == 0 || salt_len > SIZE_MAX - 1)
>   91:     return -1;
>   92: if ((asalt = malloc(salt_len + 4)) == NULL)
>   94:     return -1;
> 
> If (SIZE_MAX - 2) is passed to this function "asalt" will be
> malloc(1)'d and the subsequent memcpy on line 95 will segfault.
> This has no impact to bioctl but this implementation is linked to from
> the PBKDF2 Wikipedia article, and may be copied and used by others.
> 
> Regards,
> 
> Andrew
> 

thanks for reporting this. the diff below should fix the problem.

ok?

diff --git sbin/bioctl/pbkdf2.c sbin/bioctl/pbkdf2.c
index eba68ad..67ff075 100644
--- sbin/bioctl/pbkdf2.c
+++ sbin/bioctl/pbkdf2.c
@@ -87,7 +87,7 @@ pkcs5_pbkdf2(const char *pass, size_t pass_len, const char 
*salt, size_t salt_le
 
        if (rounds < 1 || key_len == 0)
                return -1;
-       if (salt_len == 0 || salt_len > SIZE_MAX - 1)
+       if (salt_len == 0 || salt_len > SIZE_MAX - 4)
                return -1;
        if ((asalt = malloc(salt_len + 4)) == NULL)
                return -1;