RE: [PHP-CVS] RE: [PHP-DEV] Re: [PHP-CVS] com php-src: Fix possible overflow in openssl_pbkdf2: ext/openssl/openssl.c

> -----Original Message----- > From: Anatol Belski [mailto:anatol.php@belski.net] > Sent: Wednesday, August 19, 2015 12:06 AM > To: 'Matt Wilmas' <php_lists@realplain.com>; 'Jakub Zelenka' > <bukka@php.net> > Cc: php-cvs@lists.php.net; 'PHP internals list' <internals@lists.php.net> > Subject: [PHP-CVS] RE: [PHP-DEV] Re: [PHP-CVS] com php-src: Fix possible > overflow in openssl_pbkdf2: ext/openssl/openssl.c > > > > > -----Original Message----- > > From: Matt Wilmas [mailto:php_lists@realplain.com] > > Sent: Tuesday, August 18, 2015 11:43 PM > > To: Jakub Zelenka <bukka@php.net> > > Cc: Anatol Belski <anatol.php@belski.net>; php-cvs@lists.php.net; PHP > > internals list <internals@lists.php.net> > > Subject: [PHP-DEV] Re: [PHP-CVS] com php-src: Fix possible overflow in > > openssl_pbkdf2: ext/openssl/openssl.c > > > > Hi Jakub, > > > > ----- Original Message ----- > > From: "Jakub Zelenka" > > Sent: Tuesday, August 18, 2015 > > > > > On Tue, Aug 18, 2015 at 9:48 PM, Matt Wilmas > > > <php_lists@realplain.com> > > > wrote: > > > > > >> Hi Anatol, Jakub, > > >> > > >> [...] > > >> > > >> The checks with zend_long vars like key_length and iterations are > > >> impossible when ZEND_LONG_MAX == INT_MAX (most if not all 32-bit, I > > >> guess). So those checks should already be removed by the compiler. > > >> > > >> > > > I was thinking that compiler should be able to optimize it out but > > > not sure about all compilers that we support. It's quite cheap so > > > can do that anyway but not sure if it's worthy it. > > > > If any compiler can't do such a simple optimization, then it's going > > to be missing many other possible optimizations as well. Read: Who > > cares about them. :-) > > > > >> But for the size_t ones, would need to check SIZEOF_SIZE_T > 4 > > >> around the macros or such. Or you could just change in the definition: > > >> > > >> if (_max < _var) > > >> > > >> to > > >> > > >> if (sizeof(_max) < sizeof(_var) && _max < _var) > > >> > > >> Which should work fine and allow the compiler to remove it easily, > > >> without any extra clutter. > > >> > > > > > > we are talking about size_t (unsigned) and int (signed) so not sure > > > how could compiler optimize it out on 32bit? Did I miss anything? > > > > sizeof(INT_literal_or_var) should be equal (not <) to > > sizeof(size_t_var) on 32-bit (if ints are 32-bit). So the condition > > becomes (4 < 4 && _max < > > _var) and is optimized out. The signedness doesn't matter, and we > > don't care in this case, but the compiler can't otherwise know that... > > > > Of course on 64-bit, only (_max < _var) remains -- (4 < 8) is removed. > > > Ok, got the idea now. But sizeof(size_t) is always > sizeof(zend_long). And for > signed, still it is simpler and safer to just use an #ifdef around. > Size_t > zend_long I mean :) Regards Anatol