Skip to content

Commit

Permalink
Merge branch 'PHP-8.3' into PHP-8.4
Browse files Browse the repository at this point in the history
* PHP-8.3:
  Fix GH-17187: unreachable program point in zend_hash
  • Loading branch information
nielsdos committed Dec 27, 2024
2 parents ef03609 + b621b3a commit 575ee23
Show file tree
Hide file tree
Showing 4 changed files with 234 additions and 26 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
93 changes: 93 additions & 0 deletions ext/xml/tests/gh17187_1.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
--TEST--
GH-17187 (unreachable program point in zend_hash)
--EXTENSIONS--
xml
--CREDITS--
chongwick
--FILE--
<?php
class ImmutableParser {
private $parser;
private $immutableData;
private $arrayCopy;

public function __construct() {
$this->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 = "<container><child/></container>";
$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)
}
}
53 changes: 53 additions & 0 deletions ext/xml/tests/gh17187_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
--TEST--
GH-17187 (unreachable program point in zend_hash)
--EXTENSIONS--
xml
--CREDITS--
chongwick
--FILE--
<?php
class ImmutableParser {
public $parser;
public $immutableData1;
public $immutableData2;

public function __construct() {
$this->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 = "<container><child/></container>";
$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)
111 changes: 85 additions & 26 deletions ext/xml/xml.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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)
{
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -770,27 +815,41 @@ 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),
ZSTR_VAL(decoded_value), ZSTR_LEN(decoded_value) + 1);
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);
}
}
} 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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 575ee23

Please sign in to comment.