diff --git a/NEWS b/NEWS
index d2222b5123c..a8268520eae 100644
--- a/NEWS
+++ b/NEWS
@@ -120,6 +120,9 @@ PHP NEWS
- Windows:
. Hardened proc_open() against cmd.exe hijacking. (cmb)
+- XML:
+ . Fixed bug GH-1718 (unreachable program point in zend_hash). (nielsdos)
+
05 Dec 2024, PHP 8.4.2
- BcMath:
diff --git a/ext/xml/tests/gh17187_1.phpt b/ext/xml/tests/gh17187_1.phpt
new file mode 100644
index 00000000000..4de08b33bdb
--- /dev/null
+++ b/ext/xml/tests/gh17187_1.phpt
@@ -0,0 +1,93 @@
+--TEST--
+GH-17187 (unreachable program point in zend_hash)
+--EXTENSIONS--
+xml
+--CREDITS--
+chongwick
+--FILE--
+parser = xml_parser_create();
+ xml_set_element_handler($this->parser, function ($parser, $name, $attrs) {
+ echo "open\n";
+ var_dump($name, $attrs);
+ $this->arrayCopy = [$this]; // Create cycle intentionally
+ $this->immutableData = $this->arrayCopy;
+ }, function ($parser, $name) {
+ echo "close\n";
+ var_dump($name);
+ });
+ }
+
+ public function parseXml($xml) {
+ $this->immutableData = array();
+ xml_parse_into_struct($this->parser, $xml, $this->immutableData, $this->immutableData);
+ return $this->immutableData;
+ }
+}
+$immutableParser = new ImmutableParser();
+$xml = "";
+$immutableData = $immutableParser->parseXml($xml);
+var_dump($immutableData);
+?>
+--EXPECT--
+open
+string(9) "CONTAINER"
+array(0) {
+}
+open
+string(5) "CHILD"
+array(0) {
+}
+close
+string(5) "CHILD"
+close
+string(9) "CONTAINER"
+array(5) {
+ [0]=>
+ object(ImmutableParser)#1 (3) {
+ ["parser":"ImmutableParser":private]=>
+ object(XMLParser)#2 (0) {
+ }
+ ["immutableData":"ImmutableParser":private]=>
+ *RECURSION*
+ ["arrayCopy":"ImmutableParser":private]=>
+ array(1) {
+ [0]=>
+ *RECURSION*
+ }
+ }
+ ["CHILD"]=>
+ array(1) {
+ [0]=>
+ int(1)
+ }
+ [1]=>
+ array(3) {
+ ["tag"]=>
+ string(5) "CHILD"
+ ["type"]=>
+ string(8) "complete"
+ ["level"]=>
+ int(2)
+ }
+ ["CONTAINER"]=>
+ array(1) {
+ [0]=>
+ int(2)
+ }
+ [2]=>
+ array(3) {
+ ["tag"]=>
+ string(9) "CONTAINER"
+ ["type"]=>
+ string(5) "close"
+ ["level"]=>
+ int(1)
+ }
+}
diff --git a/ext/xml/tests/gh17187_2.phpt b/ext/xml/tests/gh17187_2.phpt
new file mode 100644
index 00000000000..9a43c92cefc
--- /dev/null
+++ b/ext/xml/tests/gh17187_2.phpt
@@ -0,0 +1,53 @@
+--TEST--
+GH-17187 (unreachable program point in zend_hash)
+--EXTENSIONS--
+xml
+--CREDITS--
+chongwick
+--FILE--
+parser = xml_parser_create();
+ xml_set_element_handler($this->parser, function ($parser, $name, $attrs) {
+ echo "open\n";
+ var_dump($name, $attrs);
+ $this->immutableData1 = 0xdead;
+ $this->immutableData2 = 0xbeef;
+ }, function ($parser, $name) {
+ echo "close\n";
+ var_dump($name);
+ });
+ }
+
+ public function parseXml($xml) {
+ $this->immutableData1 = array();
+ $this->immutableData2 = array();
+ xml_parse_into_struct($this->parser, $xml, $this->immutableData1, $this->immutableData2);
+ }
+}
+$immutableParser = new ImmutableParser();
+$xml = "";
+$immutableParser->parseXml($xml);
+var_dump($immutableParser->immutableData1);
+var_dump($immutableParser->immutableData2);
+?>
+--EXPECT--
+open
+string(9) "CONTAINER"
+array(0) {
+}
+open
+string(5) "CHILD"
+array(0) {
+}
+close
+string(5) "CHILD"
+close
+string(9) "CONTAINER"
+int(57005)
+int(48879)
diff --git a/ext/xml/xml.c b/ext/xml/xml.c
index 7eca5c0795e..fb1aa2f74bb 100644
--- a/ext/xml/xml.c
+++ b/ext/xml/xml.c
@@ -79,13 +79,13 @@ typedef struct {
zend_fcall_info_cache externalEntityRefHandler;
zend_fcall_info_cache startNamespaceDeclHandler;
zend_fcall_info_cache endNamespaceDeclHandler;
-
zval data;
zval info;
+
int level;
int toffset;
int curtag;
- zval *ctag;
+ zend_long ctag_index;
char **ltags;
bool lastwasopen;
bool skipwhite;
@@ -333,6 +333,8 @@ static void xml_parser_free_obj(zend_object *object)
{
xml_parser *parser = xml_parser_from_obj(object);
+ zval_ptr_dtor(&parser->info);
+ zval_ptr_dtor(&parser->data);
if (parser->parser) {
XML_ParserFree(parser->parser);
}
@@ -425,6 +427,8 @@ static HashTable *xml_parser_get_gc(zend_object *object, zval **table, int *n)
if (ZEND_FCC_INITIALIZED(parser->endNamespaceDeclHandler)) {
zend_get_gc_buffer_add_fcc(gc_buffer, &parser->endNamespaceDeclHandler);
}
+ zend_get_gc_buffer_add_zval(gc_buffer, &parser->data);
+ zend_get_gc_buffer_add_zval(gc_buffer, &parser->info);
zend_get_gc_buffer_use(gc_buffer, table, n);
@@ -553,15 +557,18 @@ static void xml_add_to_info(xml_parser *parser, const char *name)
{
zval *element;
- if (Z_ISUNDEF(parser->info)) {
+ if (Z_ISUNDEF(parser->info) || UNEXPECTED(Z_TYPE_P(Z_REFVAL(parser->info)) != IS_ARRAY)) {
return;
}
+ SEPARATE_ARRAY(Z_REFVAL(parser->info));
+ zend_array *arr = Z_ARRVAL_P(Z_REFVAL(parser->info));
+
size_t name_len = strlen(name);
- if ((element = zend_hash_str_find(Z_ARRVAL(parser->info), name, name_len)) == NULL) {
+ if ((element = zend_hash_str_find(arr, name, name_len)) == NULL) {
zval values;
array_init(&values);
- element = zend_hash_str_update(Z_ARRVAL(parser->info), name, name_len, &values);
+ element = zend_hash_str_update(arr, name, name_len, &values);
}
add_next_index_long(element, parser->curtag);
@@ -585,6 +592,28 @@ static zend_string *xml_decode_tag(xml_parser *parser, const XML_Char *tag)
}
/* }}} */
+static zval *xml_get_separated_data(xml_parser *parser)
+{
+ if (EXPECTED(Z_TYPE_P(Z_REFVAL(parser->data)) == IS_ARRAY)) {
+ SEPARATE_ARRAY(Z_REFVAL(parser->data));
+ return Z_REFVAL(parser->data);
+ }
+ return NULL;
+}
+
+static zval *xml_get_ctag(xml_parser *parser)
+{
+ zval *data = xml_get_separated_data(parser);
+ if (EXPECTED(data)) {
+ zval *zv = zend_hash_index_find_deref(Z_ARRVAL_P(data), parser->ctag_index);
+ if (EXPECTED(zv && Z_TYPE_P(zv) == IS_ARRAY)) {
+ SEPARATE_ARRAY(zv);
+ return zv;
+ }
+ }
+ return NULL;
+}
+
/* {{{ xml_startElementHandler() */
void xml_startElementHandler(void *userData, const XML_Char *name, const XML_Char **attributes)
{
@@ -666,7 +695,19 @@ void xml_startElementHandler(void *userData, const XML_Char *name, const XML_Cha
zval_ptr_dtor(&atr);
}
- parser->ctag = zend_hash_next_index_insert(Z_ARRVAL(parser->data), &tag);
+ zval *data = xml_get_separated_data(parser);
+ if (EXPECTED(data)) {
+ /* Note: due to array resizes or user interference,
+ * we have to store an index instead of a zval into the array's memory. */
+ zend_array *arr = Z_ARRVAL_P(data);
+ if (EXPECTED(zend_hash_next_index_insert(arr, &tag))) {
+ parser->ctag_index = arr->nNextFreeElement - 1;
+ } else {
+ zval_ptr_dtor(&tag);
+ }
+ } else {
+ zval_ptr_dtor(&tag);
+ }
} else if (parser->level == (XML_MAXLEVEL + 1)) {
php_error_docref(NULL, E_WARNING, "Maximum depth exceeded - Results truncated");
}
@@ -701,17 +742,21 @@ void xml_endElementHandler(void *userData, const XML_Char *name)
zval tag;
if (parser->lastwasopen) {
- add_assoc_string(parser->ctag, "type", "complete");
+ zval *zv = xml_get_ctag(parser);
+ if (EXPECTED(zv)) {
+ add_assoc_string(zv, "type", "complete");
+ }
} else {
- array_init(&tag);
-
xml_add_to_info(parser, ZSTR_VAL(tag_name) + parser->toffset);
- add_assoc_string(&tag, "tag", SKIP_TAGSTART(ZSTR_VAL(tag_name))); /* cast to avoid gcc-warning */
- add_assoc_string(&tag, "type", "close");
- add_assoc_long(&tag, "level", parser->level);
-
- zend_hash_next_index_insert(Z_ARRVAL(parser->data), &tag);
+ zval *data = xml_get_separated_data(parser);
+ if (EXPECTED(data)) {
+ array_init(&tag);
+ add_assoc_string(&tag, "tag", SKIP_TAGSTART(ZSTR_VAL(tag_name))); /* cast to avoid gcc-warning */
+ add_assoc_string(&tag, "type", "close");
+ add_assoc_long(&tag, "level", parser->level);
+ zend_hash_next_index_insert(Z_ARRVAL_P(data), &tag);
+ }
}
parser->lastwasopen = 0;
@@ -770,9 +815,15 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
}
}
if (parser->lastwasopen) {
+ zval *ctag = xml_get_ctag(parser);
+ if (UNEXPECTED(!ctag)) {
+ zend_string_release_ex(decoded_value, false);
+ return;
+ }
+
zval *myval;
/* check if the current tag already has a value - if yes append to that! */
- if ((myval = zend_hash_find(Z_ARRVAL_P(parser->ctag), ZSTR_KNOWN(ZEND_STR_VALUE)))) {
+ if ((myval = zend_hash_find(Z_ARRVAL_P(ctag), ZSTR_KNOWN(ZEND_STR_VALUE))) && Z_TYPE_P(myval) == IS_STRING) {
size_t newlen = Z_STRLEN_P(myval) + ZSTR_LEN(decoded_value);
Z_STR_P(myval) = zend_string_extend(Z_STR_P(myval), newlen, 0);
strncpy(Z_STRVAL_P(myval) + Z_STRLEN_P(myval) - ZSTR_LEN(decoded_value),
@@ -780,7 +831,7 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
zend_string_release_ex(decoded_value, 0);
} else {
if (doprint || (! parser->skipwhite)) {
- add_assoc_str(parser->ctag, "value", decoded_value);
+ add_assoc_str(ctag, "value", decoded_value);
} else {
zend_string_release_ex(decoded_value, 0);
}
@@ -788,9 +839,17 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
} else {
zval tag;
zval *curtag, *mytype, *myval;
- ZEND_HASH_REVERSE_FOREACH_VAL(Z_ARRVAL(parser->data), curtag) {
- if ((mytype = zend_hash_str_find(Z_ARRVAL_P(curtag),"type", sizeof("type") - 1))) {
- if (zend_string_equals_literal(Z_STR_P(mytype), "cdata")) {
+
+ zval *data = xml_get_separated_data(parser);
+ if (UNEXPECTED(!data)) {
+ zend_string_release_ex(decoded_value, false);
+ return;
+ }
+
+ ZEND_HASH_REVERSE_FOREACH_VAL(Z_ARRVAL_P(data), curtag) {
+ if (EXPECTED(Z_TYPE_P(curtag) == IS_ARRAY) && (mytype = zend_hash_str_find(Z_ARRVAL_P(curtag),"type", sizeof("type") - 1))) {
+ if (EXPECTED(Z_TYPE_P(mytype) == IS_STRING) && zend_string_equals_literal(Z_STR_P(mytype), "cdata")) {
+ SEPARATE_ARRAY(curtag);
if ((myval = zend_hash_find(Z_ARRVAL_P(curtag), ZSTR_KNOWN(ZEND_STR_VALUE)))) {
size_t newlen = Z_STRLEN_P(myval) + ZSTR_LEN(decoded_value);
Z_STR_P(myval) = zend_string_extend(Z_STR_P(myval), newlen, 0);
@@ -810,7 +869,7 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
add_assoc_str(&tag, "value", decoded_value);
add_assoc_string(&tag, "type", "cdata");
add_assoc_long(&tag, "level", parser->level);
- zend_hash_next_index_insert(Z_ARRVAL(parser->data), &tag);
+ zend_hash_next_index_insert(Z_ARRVAL_P(data), &tag);
} else if (parser->level == (XML_MAXLEVEL + 1)) {
php_error_docref(NULL, E_WARNING, "Maximum depth exceeded - Results truncated");
} else {
@@ -1366,21 +1425,21 @@ PHP_FUNCTION(xml_parse_into_struct)
}
if (info) {
- info = zend_try_array_init(info);
- if (!info) {
+ if (!zend_try_array_init(info)) {
RETURN_THROWS();
}
}
- xdata = zend_try_array_init(xdata);
- if (!xdata) {
+ if (!zend_try_array_init(xdata)) {
RETURN_THROWS();
}
- ZVAL_COPY_VALUE(&parser->data, xdata);
+ zval_ptr_dtor(&parser->data);
+ ZVAL_COPY(&parser->data, xdata);
if (info) {
- ZVAL_COPY_VALUE(&parser->info, info);
+ zval_ptr_dtor(&parser->info);
+ ZVAL_COPY(&parser->info, info);
}
parser->level = 0;