diff --git a/ext/xml/tests/gh17187_1.phpt b/ext/xml/tests/gh17187_1.phpt new file mode 100644 index 0000000000000..4de08b33bdb81 --- /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 0000000000000..9a43c92cefc03 --- /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 73b51b2143df9..62507a5d1307f 100644 --- a/ext/xml/xml.c +++ b/ext/xml/xml.c @@ -72,7 +72,7 @@ typedef struct { /* We return a pointer to these zvals in get_gc(), so it's * important that a) they are adjacent b) object is the first * and c) the number of zvals is kept up to date. */ -#define XML_PARSER_NUM_ZVALS 12 +#define XML_PARSER_NUM_ZVALS 14 zval object; zval startElementHandler; zval endElementHandler; @@ -85,6 +85,8 @@ typedef struct { zval unknownEncodingHandler; zval startNamespaceDeclHandler; zval endNamespaceDeclHandler; + zval data; + zval info; zend_function *startElementPtr; zend_function *endElementPtr; @@ -98,12 +100,10 @@ typedef struct { zend_function *startNamespaceDeclPtr; zend_function *endNamespaceDeclPtr; - zval data; - zval info; int level; int toffset; int curtag; - zval *ctag; + zend_long ctag_index; char **ltags; int lastwasopen; int skipwhite; @@ -326,6 +326,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); } @@ -551,15 +553,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); @@ -583,6 +588,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) { @@ -662,7 +689,19 @@ void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Ch 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"); } @@ -697,17 +736,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; @@ -765,9 +808,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), @@ -775,7 +824,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); } @@ -783,9 +832,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); @@ -805,7 +862,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 { @@ -1266,21 +1323,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;