[prev in list] [next in list] [prev in thread] [next in thread] 

List:       php-internals
Subject:    Re: [PHP-DEV] Asking for a review of crypt() allocation changes
From:       Ángel_González <keisial () gmail ! com>
Date:       2012-06-30 7:38:06
Message-ID: 4FEEACDE.5020008 () gmail ! com
[Download RAW message or body]


On 29/06/12 14:43, Nikita Popov wrote:
> Hi internals!
>
> Anthony and me have been looking a lot at the crypt() code recently
> and noticed that there are some strange things going on in the buffer
> allocations for the sha algorithms.
>
> We did two commits to fix them up a bit:
>
> http://git.php.net/?p=php-src.git;a=commitdiff;h=7e8276ca68fc622124d51d18e4f7b5cde3536de4
It took me a while to realise the problem being fixed. The bug is not
the memset (as reported in bug 62443),
which is using needed (got fixed in e6cf7d), or php_sha{256,512}_crypt_r
(uses a null-terminated string), but the salt[salt_in_len] = '\0';
after allocating only strlen(salt).
So that you would be accessing the position PHP_MAX_SALT_LEN of the
array but have reserved only a few bytes.
Just*sizeof*(sha512_rounds_prefix
<http://lxr.php.net/xref/PHP_TRUNK/ext/standard/crypt.c#sha512_rounds_prefix>)
+ 9 + 1 seem enough for not making bug62443.phpt segfault.
I have been able to crash it with var_dump( crypt("foo", '$6$'.chr(0).
str_pad('', 500, '*') . '$abc') );
but only if it's the first call.




[prev in list] [next in list] [prev in thread] [next in thread]