From 6d5850c931d95a61de1d3553b913f03dc00d8d5f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 11 Apr 2025 18:18:14 +0200 Subject: [PATCH] Fix GH-18304: Changing the properties of a DateInterval through dynamic properties triggers a SegFault For dynamic fetches the cache_slot will be NULL, so we have to check for that when resetting the cache. For zip and xmlreader this couldn't easily be tested because of a lack of writable properties. --- ext/date/php_date.c | 4 +++- ext/date/tests/gh18304.phpt | 35 ++++++++++++++++++++++++++++++++ ext/dom/php_dom.c | 4 +++- ext/dom/tests/gh18304.phpt | 15 ++++++++++++++ ext/pdo/pdo_stmt.c | 5 +++-- ext/simplexml/simplexml.c | 4 +++- ext/simplexml/tests/gh18304.phpt | 18 ++++++++++++++++ ext/snmp/snmp.c | 4 +++- ext/snmp/tests/gh18304.phpt | 15 ++++++++++++++ ext/spl/spl_array.c | 4 +++- ext/spl/tests/gh18304.phpt | 14 +++++++++++++ ext/xmlreader/php_xmlreader.c | 4 ++-- ext/zip/php_zip.c | 4 ++-- 13 files changed, 119 insertions(+), 11 deletions(-) create mode 100644 ext/date/tests/gh18304.phpt create mode 100644 ext/dom/tests/gh18304.phpt create mode 100644 ext/simplexml/tests/gh18304.phpt create mode 100644 ext/snmp/tests/gh18304.phpt create mode 100644 ext/spl/tests/gh18304.phpt diff --git a/ext/date/php_date.c b/ext/date/php_date.c index 6f7796eec1250..c1faa842f8a12 100644 --- a/ext/date/php_date.c +++ b/ext/date/php_date.c @@ -4399,7 +4399,9 @@ static zval *date_interval_get_property_ptr_ptr(zend_object *object, zend_string zend_string_equals_literal(name, "days") || zend_string_equals_literal(name, "invert") ) { /* Fallback to read_property. */ - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + } ret = NULL; } else { ret = zend_std_get_property_ptr_ptr(object, name, type, cache_slot); diff --git a/ext/date/tests/gh18304.phpt b/ext/date/tests/gh18304.phpt new file mode 100644 index 0000000000000..4bab058a7ec4a --- /dev/null +++ b/ext/date/tests/gh18304.phpt @@ -0,0 +1,35 @@ +--TEST-- +GH-18304 (Changing the properties of a DateInterval through dynamic properties triggers a SegFault) +--CREDITS-- +orose-assetgo +--FILE-- +$field += $i; +var_dump($di); +?> +--EXPECT-- +object(DateInterval)#1 (10) { + ["y"]=> + int(0) + ["m"]=> + int(0) + ["d"]=> + int(1) + ["h"]=> + int(0) + ["i"]=> + int(0) + ["s"]=> + int(0) + ["f"]=> + float(0) + ["invert"]=> + int(0) + ["days"]=> + bool(false) + ["from_string"]=> + bool(false) +} diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 1da53bae64b51..841dcd66186f8 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -303,7 +303,9 @@ static zval *dom_get_property_ptr_ptr(zend_object *object, zend_string *name, in return zend_std_get_property_ptr_ptr(object, name, type, cache_slot); } - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + } return NULL; } diff --git a/ext/dom/tests/gh18304.phpt b/ext/dom/tests/gh18304.phpt new file mode 100644 index 0000000000000..ead44306ff801 --- /dev/null +++ b/ext/dom/tests/gh18304.phpt @@ -0,0 +1,15 @@ +--TEST-- +GH-18304 (Changing the properties of a DateInterval through dynamic properties triggers a SegFault) +--CREDITS-- +orose-assetgo +--EXTENSIONS-- +dom +--FILE-- +$field .= 'hello'; +var_dump($text->$field); +?> +--EXPECT-- +string(5) "hello" diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index efbf519e5411e..8454b4875374e 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2493,9 +2493,10 @@ static zval *pdo_row_get_property_ptr_ptr(zend_object *object, zend_string *name ZEND_IGNORE_VALUE(object); ZEND_IGNORE_VALUE(name); ZEND_IGNORE_VALUE(type); - ZEND_IGNORE_VALUE(cache_slot); - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + } return NULL; } diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 46ec3a2a788ab..11f497a6673ea 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -635,7 +635,9 @@ static zval *sxe_property_get_adr(zend_object *object, zend_string *zname, int f SXE_ITER type; zval member; - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + } sxe = php_sxe_fetch_object(object); GET_NODE(sxe, node); diff --git a/ext/simplexml/tests/gh18304.phpt b/ext/simplexml/tests/gh18304.phpt new file mode 100644 index 0000000000000..b92690856e231 --- /dev/null +++ b/ext/simplexml/tests/gh18304.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-18304 (Changing the properties of a DateInterval through dynamic properties triggers a SegFault) +--CREDITS-- +orose-assetgo +--EXTENSIONS-- +simplexml +--FILE-- +'); +$field = 'abc'; +$sxe->$field .= 'hello'; +var_dump($sxe->$field); +?> +--EXPECT-- +object(SimpleXMLElement)#3 (1) { + [0]=> + string(5) "hello" +} diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index 02348a5fbca00..0ba39ed87c241 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -1861,7 +1861,9 @@ static zval *php_snmp_get_property_ptr_ptr(zend_object *object, zend_string *nam return zend_std_get_property_ptr_ptr(object, name, type, cache_slot); } - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + } return NULL; } diff --git a/ext/snmp/tests/gh18304.phpt b/ext/snmp/tests/gh18304.phpt new file mode 100644 index 0000000000000..2faf503ac1a58 --- /dev/null +++ b/ext/snmp/tests/gh18304.phpt @@ -0,0 +1,15 @@ +--TEST-- +GH-18304 (Changing the properties of a DateInterval through dynamic properties triggers a SegFault) +--CREDITS-- +orose-assetgo +--EXTENSIONS-- +snmp +--FILE-- +$field++; +var_dump($snmp->$field); +?> +--EXPECT-- +int(1) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 8d4541797a1c5..0abaf743ac9d1 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -846,7 +846,9 @@ static zval *spl_array_get_property_ptr_ptr(zend_object *object, zend_string *na if ((intern->ar_flags & SPL_ARRAY_ARRAY_AS_PROPS) != 0 && !zend_std_has_property(object, name, ZEND_PROPERTY_EXISTS, NULL)) { - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; + } /* If object has offsetGet() overridden, then fallback to read_property, * which will call offsetGet(). */ diff --git a/ext/spl/tests/gh18304.phpt b/ext/spl/tests/gh18304.phpt new file mode 100644 index 0000000000000..d93ee3534d0d4 --- /dev/null +++ b/ext/spl/tests/gh18304.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-18304 (Changing the properties of a DateInterval through dynamic properties triggers a SegFault) +--CREDITS-- +orose-assetgo +--FILE-- + 1]); +$ao->setFlags(ArrayObject::ARRAY_AS_PROPS); +$field = 'abc'; +$ao->$field++; +var_dump($ao->$field); +?> +--EXPECT-- +int(2) diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c index 569058e91a454..dca1898f1c044 100644 --- a/ext/xmlreader/php_xmlreader.c +++ b/ext/xmlreader/php_xmlreader.c @@ -121,8 +121,6 @@ zval *xmlreader_get_property_ptr_ptr(zend_object *object, zend_string *name, int zval *retval = NULL; xmlreader_prop_handler *hnd = NULL; - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; - obj = php_xmlreader_fetch_object(object); if (obj->prop_handler != NULL) { @@ -131,6 +129,8 @@ zval *xmlreader_get_property_ptr_ptr(zend_object *object, zend_string *name, int if (hnd == NULL) { retval = zend_std_get_property_ptr_ptr(object, name, type, cache_slot); + } else if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; } return retval; diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index e28cf688dcff8..172057685105b 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -890,8 +890,6 @@ static zval *php_zip_get_property_ptr_ptr(zend_object *object, zend_string *name zval *retval = NULL; zip_prop_handler *hnd = NULL; - cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; - obj = php_zip_fetch_object(object); if (obj->prop_handler != NULL) { @@ -900,6 +898,8 @@ static zval *php_zip_get_property_ptr_ptr(zend_object *object, zend_string *name if (hnd == NULL) { retval = zend_std_get_property_ptr_ptr(object, name, type, cache_slot); + } else if (cache_slot) { + cache_slot[0] = cache_slot[1] = cache_slot[2] = NULL; } return retval;