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. Regards Anatol