Re: com php-src: Fix possible overflow in openssl_pbkdf2: ext/openssl/openssl.c

On Tue, Aug 18, 2015 at 8:23 PM, Anatol Belski <anatol.php@belski.net> wrote: > Hi Jakub, > > > -----Original Message----- > > From: Jakub Zelenka [mailto:bukka@php.net] > > Sent: Tuesday, August 18, 2015 8:47 PM > > To: php-cvs@lists.php.net > > Subject: [PHP-CVS] com php-src: Fix possible overflow in openssl_pbkdf2: > > ext/openssl/openssl.c > > > > Commit: 618c327a56b03449324cdaa0d630ea710aea22fd > > Author: Jakub Zelenka <bukka@php.net> Tue, 18 Aug 2015 > 19:46:59 +0100 > > Parents: 000be61fb8a034b3ce2dc3d7bbfbc912295b86e9 > > Branches: master > > > > Link: http://git.php.net/?p=php- > > src.git;a=commitdiff;h=618c327a56b03449324cdaa0d630ea710aea22fd > > > > Log: > > Fix possible overflow in openssl_pbkdf2 > > > > Especially key_length would lead to the crash if it overflowed to the > negative > > value. > > > > Changed paths: > > M ext/openssl/openssl.c > > > > > > Diff: > > diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index > > 1608e5d..1e03ce7 100644 > > --- a/ext/openssl/openssl.c > > +++ b/ext/openssl/openssl.c > > @@ -4011,6 +4011,22 @@ PHP_FUNCTION(openssl_pbkdf2) > > if (key_length <= 0) { > > RETURN_FALSE; > > } > > + if (INT_MAX < key_length) { > > + php_error_docref(NULL, E_WARNING, "key_length is too > > long"); > > + RETURN_FALSE; > > + } > Key_length is probably not going to be bigger than INT_MAX on 32-bit. I > guess there are other similar cases. IMHO it'd make sense to #ifdef them to > avoid overheads (even if small) on 64-bit. > Yeah these are just for 64bit where it can be bigger. I just wrapped all checks with macros so I can split them for size_t -> int and long -> int and make the letter noop on 32-bit. However it's really really small overhead compare to other bits in the function. :) But good point anyway ;) Cheers