Re: com php-src: fix signed/unsigned mismatch: Zend/zend_execute.c

On Mon, Sep 15, 2014 at 12:58 PM, Anatol Belski <ab@php.net> wrote: > Commit: 836fd73cce8d0550baf5477bfb0ea0edbfae455a > Author: Anatol Belski <ab@php.net> Mon, 15 Sep 2014 12:12:18 > +0200 > Parents: c8ed0d81e755c700f86cddb74f62eda27bc6e2de > Branches: master > > Link: > > http://git.php.net/?p=php-src.git;a=commitdiff;h=836fd73cce8d0550baf5477bfb0ea0edbfae455a > > Log: > fix signed/unsigned mismatch > > Changed paths: > M Zend/zend_execute.c > > > Diff: > diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c > index 27636fd..5e774e2 100644 > --- a/Zend/zend_execute.c > +++ b/Zend/zend_execute.c > @@ -771,7 +771,7 @@ static void zend_assign_to_string_offset(zval *str, > zend_long offset, zval *valu > } > > old_str = Z_STR_P(str); > - if (offset >= Z_STRLEN_P(str)) { > + if (offset >= (zend_long)Z_STRLEN_P(str)) { > zend_long old_len = Z_STRLEN_P(str); > Z_STR_P(str) = zend_string_realloc(Z_STR_P(str), offset + > 1, 0); > Z_TYPE_INFO_P(str) = IS_STRING_EX; > @@ -1226,7 +1226,7 @@ static zend_always_inline void > zend_fetch_dimension_address_read(zval *result, z > offset = Z_LVAL_P(dim); > } > > - if (UNEXPECTED(offset < 0) || > UNEXPECTED(Z_STRLEN_P(container) <= offset)) { > + if (UNEXPECTED(offset < 0) || > UNEXPECTED((zend_long)Z_STRLEN_P(container) <= offset)) { > if (type != BP_VAR_IS) { > zend_error(E_NOTICE, "Uninitialized string > offset: %pd", offset); > } > I wonder if these casts shouldn't happen the other way around, i.e. whether offset should be cast to size_t instead of casting the length to zend_long. The case I'm thinking about is this: If len > ZEND_LONG_MAX, then (zend_long)len will be < 0, so offset will always be >= than (zend_long)len in this case, leading to out of bounds memory access. If we cast the offset to (size_t) instead this shouldn't happen. Nikita