Re: [PHP-CVS] com php-src: Fixed bug #63882 (zend_std_compare_objects crash on recursion): NEWS Zend/tests/bug63882.phpt Zend/zend_object_handlers.c Zend/zend_objects_API.c Zend/zend_objects_API.h

2013-01-10 Thread Dmitry Stogov
Hi Stas,

Actually this change in zend_object structure doesn't affect the previous
structure layout,
because of field alignment. The new 1 byte field created inside a 2 byte
hole that wasn't used before.

Thanks. Dmitry,

On Wed, Jan 9, 2013 at 8:53 PM, Stas Malyshev wrote:

> Hi!
>
> Doesn't change of zend_objects_API.h change binary API? I'm not sure we
> can do it in 5.4.
>
> On 1/8/13 11:30 PM, Dmitry Stogov wrote:
> > Commit:f9e8678dd3a41ed8a100d8201153a41d6fd25f2e
> > Author:Dmitry Stogov  Wed, 9 Jan 2013
> 11:30:50 +0400
> > Parents:   f3b1b8516906fe900e521216c8f01e362790af30
> > Branches:  PHP-5.4 PHP-5.5 master
> >
> > Link:
> http://git.php.net/?p=php-src.git;a=commitdiff;h=f9e8678dd3a41ed8a100d8201153a41d6fd25f2e
> >
> > Log:
> > Fixed bug #63882 (zend_std_compare_objects crash on recursion)
> >
> > Bugs:
> > https://bugs.php.net/63882
> >
> > Changed paths:
> >   M  NEWS
> >   A  Zend/tests/bug63882.phpt
> >   M  Zend/zend_object_handlers.c
> >   M  Zend/zend_objects_API.c
> >   M  Zend/zend_objects_API.h
> >
> >
> > Diff:
> > diff --git a/NEWS b/NEWS
> > index 1abd398..cfc0fa9 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -5,6 +5,7 @@ PHP
>NEWS
> >  - Core:
> >. Fixed bug #63943 (Bad warning text from strpos() on empty needle).
> >  (Laruence)
> > +  . Fixed bug #63882 (zend_std_compare_objects crash on recursion).
> (Dmitry)
> >
> >  - Litespeed:
> >. Fixed bug #63228 (-Werror=format-security error in lsapi code).
> (George)
> > diff --git a/Zend/tests/bug63882.phpt b/Zend/tests/bug63882.phpt
> > new file mode 100644
> > index 000..0cc1bab
> > --- /dev/null
> > +++ b/Zend/tests/bug63882.phpt
> > @@ -0,0 +1,15 @@
> > +--TEST--
> > +Bug #63882 (zend_std_compare_objects crash on recursion)
> > +--FILE--
> > + > +class Test { public $x = 5; }
> > +
> > +$testobj1 = new Test;
> > +$testobj2 = new Test;
> > +$testobj1->x = $testobj1;
> > +$testobj2->x = $testobj2;
> > +
> > +var_dump($testobj1 == $testobj2);
> > +?>
> > +--EXPECTF--
> > +Fatal error: Nesting level too deep - recursive dependency? in
> %sbug63882.php on line 9
> > diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
> > index a76dfb3..3881c0e 100644
> > --- a/Zend/zend_object_handlers.c
> > +++ b/Zend/zend_object_handlers.c
> > @@ -35,6 +35,17 @@
> >  #define Z_OBJ_P(zval_p) \
> >
> ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(zval_p)].bucket.obj.object))
> >
> > +#define Z_OBJ_PROTECT_RECURSION(zval_p) \
> > + do { \
> > + if
> (EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(zval_p)].apply_count++ >=
> 3) { \
> > + zend_error(E_ERROR, "Nesting level too deep -
> recursive dependency?"); \
> > + } \
> > + } while (0)
> > +
> > +
> > +#define Z_OBJ_UNPROTECT_RECURSION(zval_p) \
> > +
> EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(zval_p)].apply_count--
> > +
> >  /*
> >__X accessors explanation:
> >
> > @@ -1319,28 +1330,43 @@ static int zend_std_compare_objects(zval *o1,
> zval *o2 TSRMLS_DC) /* {{{ */
> >   }
> >   if (!zobj1->properties && !zobj2->properties) {
> >   int i;
> > +
> > + Z_OBJ_PROTECT_RECURSION(o1);
> > + Z_OBJ_PROTECT_RECURSION(o2);
> >   for (i = 0; i < zobj1->ce->default_properties_count; i++) {
> >   if (zobj1->properties_table[i]) {
> >   if (zobj2->properties_table[i]) {
> >   zval result;
> >
> >   if (compare_function(&result,
> zobj1->properties_table[i], zobj2->properties_table[i] TSRMLS_CC)==FAILURE)
> {
> > +
> Z_OBJ_UNPROTECT_RECURSION(o1);
> > +
> Z_OBJ_UNPROTECT_RECURSION(o2);
> >   return 1;
> >   }
> >   if (Z_LVAL(result) != 0) {
> > +
> Z_OBJ_UNPROTECT_RECURSION(o1);
> > +
> Z_OBJ_UNPROTECT_RECURSION(o2);
> >   return Z_LVAL(result);
> >   }
> >   } else {
> > + Z_OBJ_UNPROTECT_RECURSION(o1);
> > + Z_OBJ_UNPROTECT_RECURSION(o2);
> >   return 1;
> >   }
> >   } else {
> >   if (zobj2->properties_table[i]) {
> > + Z_OBJ_UNPROTECT_RECURSION(o1);
> > + Z_OBJ_UNPROTECT_RECURSION(o2);
> >   return 1;
> >   } else {
> > + Z_OBJ_UNPROTECT_RECURSION(o1);
> > + Z_OBJ_UNPROTECT_RECURSION(o2);
> >   return 0;
> >   }
> >

Re: [PHP-CVS] com php-src: Fixed bug #63882 (zend_std_compare_objects crash on recursion): NEWS Zend/tests/bug63882.phpt Zend/zend_object_handlers.c Zend/zend_objects_API.c Zend/zend_objects_API.h

2013-01-09 Thread Stas Malyshev
Hi!

Doesn't change of zend_objects_API.h change binary API? I'm not sure we
can do it in 5.4.

On 1/8/13 11:30 PM, Dmitry Stogov wrote:
> Commit:f9e8678dd3a41ed8a100d8201153a41d6fd25f2e
> Author:Dmitry Stogov  Wed, 9 Jan 2013 11:30:50 
> +0400
> Parents:   f3b1b8516906fe900e521216c8f01e362790af30
> Branches:  PHP-5.4 PHP-5.5 master
> 
> Link:   
> http://git.php.net/?p=php-src.git;a=commitdiff;h=f9e8678dd3a41ed8a100d8201153a41d6fd25f2e
> 
> Log:
> Fixed bug #63882 (zend_std_compare_objects crash on recursion)
> 
> Bugs:
> https://bugs.php.net/63882
> 
> Changed paths:
>   M  NEWS
>   A  Zend/tests/bug63882.phpt
>   M  Zend/zend_object_handlers.c
>   M  Zend/zend_objects_API.c
>   M  Zend/zend_objects_API.h
> 
> 
> Diff:
> diff --git a/NEWS b/NEWS
> index 1abd398..cfc0fa9 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,7 @@ PHP   
>  NEWS
>  - Core:
>. Fixed bug #63943 (Bad warning text from strpos() on empty needle).
>  (Laruence)
> +  . Fixed bug #63882 (zend_std_compare_objects crash on recursion). (Dmitry)
>  
>  - Litespeed:
>. Fixed bug #63228 (-Werror=format-security error in lsapi code). (George)
> diff --git a/Zend/tests/bug63882.phpt b/Zend/tests/bug63882.phpt
> new file mode 100644
> index 000..0cc1bab
> --- /dev/null
> +++ b/Zend/tests/bug63882.phpt
> @@ -0,0 +1,15 @@
> +--TEST--
> +Bug #63882 (zend_std_compare_objects crash on recursion)
> +--FILE--
> + +class Test { public $x = 5; }
> +
> +$testobj1 = new Test;
> +$testobj2 = new Test;
> +$testobj1->x = $testobj1;
> +$testobj2->x = $testobj2;
> +
> +var_dump($testobj1 == $testobj2);
> +?>
> +--EXPECTF--
> +Fatal error: Nesting level too deep - recursive dependency? in 
> %sbug63882.php on line 9
> diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
> index a76dfb3..3881c0e 100644
> --- a/Zend/zend_object_handlers.c
> +++ b/Zend/zend_object_handlers.c
> @@ -35,6 +35,17 @@
>  #define Z_OBJ_P(zval_p) \
>   
> ((zend_object*)(EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(zval_p)].bucket.obj.object))
>  
> +#define Z_OBJ_PROTECT_RECURSION(zval_p) \
> + do { \
> + if 
> (EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(zval_p)].apply_count++ >= 3) 
> { \
> + zend_error(E_ERROR, "Nesting level too deep - recursive 
> dependency?"); \
> + } \
> + } while (0)
> +
> +
> +#define Z_OBJ_UNPROTECT_RECURSION(zval_p) \
> + EG(objects_store).object_buckets[Z_OBJ_HANDLE_P(zval_p)].apply_count--
> +
>  /*
>__X accessors explanation:
>  
> @@ -1319,28 +1330,43 @@ static int zend_std_compare_objects(zval *o1, zval 
> *o2 TSRMLS_DC) /* {{{ */
>   }
>   if (!zobj1->properties && !zobj2->properties) {
>   int i;
> +
> + Z_OBJ_PROTECT_RECURSION(o1);
> + Z_OBJ_PROTECT_RECURSION(o2);
>   for (i = 0; i < zobj1->ce->default_properties_count; i++) {
>   if (zobj1->properties_table[i]) {
>   if (zobj2->properties_table[i]) {
>   zval result;
>  
>   if (compare_function(&result, 
> zobj1->properties_table[i], zobj2->properties_table[i] TSRMLS_CC)==FAILURE) {
> + Z_OBJ_UNPROTECT_RECURSION(o1);
> + Z_OBJ_UNPROTECT_RECURSION(o2);
>   return 1;
>   }
>   if (Z_LVAL(result) != 0) {
> + Z_OBJ_UNPROTECT_RECURSION(o1);
> + Z_OBJ_UNPROTECT_RECURSION(o2);
>   return Z_LVAL(result);
>   }
>   } else {
> + Z_OBJ_UNPROTECT_RECURSION(o1);
> + Z_OBJ_UNPROTECT_RECURSION(o2);
>   return 1;
>   }
>   } else {
>   if (zobj2->properties_table[i]) {
> + Z_OBJ_UNPROTECT_RECURSION(o1);
> + Z_OBJ_UNPROTECT_RECURSION(o2);
>   return 1;
>   } else {
> + Z_OBJ_UNPROTECT_RECURSION(o1);
> + Z_OBJ_UNPROTECT_RECURSION(o2);
>   return 0;
>   }
>   }
>   }
> + Z_OBJ_UNPROTECT_RECURSION(o1);
> + Z_OBJ_UNPROTECT_RECURSION(o2);
>   return 0;
>   } else {
>   if (!zobj1->properties) {
> diff --git a/Zend/zend_objects_API.c b/Zend/z