Skip to content

Commit

Permalink
Fix #80332: Completely broken array access functionality with DOMName…
Browse files Browse the repository at this point in the history 
…dNodeMap

The problem is the usage of zval_get_long(). In particular, if the
string is non-numeric the result of zval_get_long() will be 0 without
giving an error or warning. This is misleading for users: users get the
impression that they can use strings to access the map because it
coincidentally works for the first item (which is at index 0). Of
course, this fails with any other index which causes confusion and bugs.

This patch adds proper support for using string offsets while accessing
the map. It does so by detecting if it's a non-numeric string, and then
using the getNamedItem() method instead of item(). I had to split up the
array access implementation code for DOMNodeList and DOMNamedNodeMap
first to be able to do this.

Closes GH-11468.
  • Loading branch information
@nielsdos
nielsdos committed Jun 18, 2023
1 parent f194cdf commit 9f7d888
Show file tree
Hide file tree
Showing 8 changed files with 318 additions and 107 deletions.
2 changes: 2 additions & 0 deletions NEWS
View file
Expand Up @@ -40,6 +40,8 @@ PHP NEWS
issues). (nielsdos)
. Fixed bug GH-11404 (DOMDocument::saveXML and friends omit xmlns=""
declaration for null namespace). (nielsdos)
. Fixed bug #80332 (Completely broken array access functionality with
DOMNamedNodeMap). (nielsdos)

- Opcache:
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)
Expand Down
122 changes: 63 additions & 59 deletions ext/dom/namednodemap.c
View file
Expand Up @@ -31,7 +31,7 @@
* Since:
*/

static int get_namednodemap_length(dom_object *obj)
int php_dom_get_namednodemap_length(dom_object *obj)
{
dom_nnodemap_object *objmap = (dom_nnodemap_object *) obj->ptr;
if (!objmap) {
Expand Down Expand Up @@ -65,95 +65,74 @@ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-
*/
int dom_namednodemap_length_read(dom_object *obj, zval *retval)
{
ZVAL_LONG(retval, get_namednodemap_length(obj));
ZVAL_LONG(retval, php_dom_get_namednodemap_length(obj));
return SUCCESS;
}

/* }}} */

/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-1074577549
Since:
*/
PHP_METHOD(DOMNamedNodeMap, getNamedItem)
xmlNodePtr php_dom_named_node_map_get_named_item(dom_nnodemap_object *objmap, const char *named, bool may_transform)
{
zval *id;
int ret;
size_t namedlen=0;
dom_object *intern;
xmlNodePtr itemnode = NULL;
char *named;

dom_nnodemap_object *objmap;
xmlNodePtr nodep;
xmlNotation *notep = NULL;

id = ZEND_THIS;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &named, &namedlen) == FAILURE) {
RETURN_THROWS();
}

intern = Z_DOMOBJ_P(id);

objmap = (dom_nnodemap_object *)intern->ptr;

if (objmap != NULL) {
if ((objmap->nodetype == XML_NOTATION_NODE) ||
objmap->nodetype == XML_ENTITY_NODE) {
if (objmap->ht) {
if (objmap->nodetype == XML_ENTITY_NODE) {
itemnode = (xmlNodePtr)xmlHashLookup(objmap->ht, (xmlChar *) named);
itemnode = (xmlNodePtr)xmlHashLookup(objmap->ht, (const xmlChar *) named);
} else {
notep = (xmlNotation *)xmlHashLookup(objmap->ht, (xmlChar *) named);
xmlNotationPtr notep = xmlHashLookup(objmap->ht, (const xmlChar *) named);
if (notep) {
itemnode = create_notation(notep->name, notep->PublicID, notep->SystemID);
if (may_transform) {
itemnode = create_notation(notep->name, notep->PublicID, notep->SystemID);
} else {
itemnode = (xmlNodePtr) notep;
}
}
}
}
} else {
nodep = dom_object_get_node(objmap->baseobj);
xmlNodePtr nodep = dom_object_get_node(objmap->baseobj);
if (nodep) {
itemnode = (xmlNodePtr)xmlHasProp(nodep, (xmlChar *) named);
itemnode = (xmlNodePtr)xmlHasProp(nodep, (const xmlChar *) named);
}
}
}
return itemnode;
}

void php_dom_named_node_map_get_named_item_into_zval(dom_nnodemap_object *objmap, const char *named, zval *return_value)
{
int ret;
xmlNodePtr itemnode = php_dom_named_node_map_get_named_item(objmap, named, true);
if (itemnode) {
DOM_RET_OBJ(itemnode, &ret, objmap->baseobj);
return;
} else {
RETVAL_NULL();
RETURN_NULL();
}
}
/* }}} end dom_namednodemap_get_named_item */

/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-349467F9
/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-1074577549
Since:
*/
PHP_METHOD(DOMNamedNodeMap, item)
PHP_METHOD(DOMNamedNodeMap, getNamedItem)
{
zval *id;
zend_long index;
int ret;
dom_object *intern;
xmlNodePtr itemnode = NULL;

dom_nnodemap_object *objmap;
xmlNodePtr nodep, curnode;
int count;
size_t namedlen;
char *named;

id = ZEND_THIS;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
RETURN_THROWS();
}
if (index < 0 || ZEND_LONG_INT_OVFL(index)) {
zend_argument_value_error(1, "must be between 0 and %d", INT_MAX);
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &named, &namedlen) == FAILURE) {
RETURN_THROWS();
}

intern = Z_DOMOBJ_P(id);

objmap = (dom_nnodemap_object *)intern->ptr;
zval *id = ZEND_THIS;
dom_nnodemap_object *objmap = Z_DOMOBJ_P(id)->ptr;
php_dom_named_node_map_get_named_item_into_zval(objmap, named, return_value);
}
/* }}} end dom_namednodemap_get_named_item */

xmlNodePtr php_dom_named_node_map_get_item(dom_nnodemap_object *objmap, zend_long index)
{
xmlNodePtr itemnode = NULL;
if (objmap != NULL) {
if ((objmap->nodetype == XML_NOTATION_NODE) ||
objmap->nodetype == XML_ENTITY_NODE) {
Expand All @@ -165,10 +144,10 @@ PHP_METHOD(DOMNamedNodeMap, item)
}
}
} else {
nodep = dom_object_get_node(objmap->baseobj);
xmlNodePtr nodep = dom_object_get_node(objmap->baseobj);
if (nodep) {
curnode = (xmlNodePtr)nodep->properties;
count = 0;
xmlNodePtr curnode = (xmlNodePtr)nodep->properties;
zend_long count = 0;
while (count < index && curnode != NULL) {
count++;
curnode = (xmlNodePtr)curnode->next;
Expand All @@ -177,13 +156,38 @@ PHP_METHOD(DOMNamedNodeMap, item)
}
}
}
return itemnode;
}

void php_dom_named_node_map_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value)
{
int ret;
xmlNodePtr itemnode = php_dom_named_node_map_get_item(objmap, index);
if (itemnode) {
DOM_RET_OBJ(itemnode, &ret, objmap->baseobj);
return;
} else {
RETURN_NULL();
}
}

/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-349467F9
Since:
*/
PHP_METHOD(DOMNamedNodeMap, item)
{
zend_long index;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
RETURN_THROWS();
}
if (index < 0 || ZEND_LONG_INT_OVFL(index)) {
zend_argument_value_error(1, "must be between 0 and %d", INT_MAX);
RETURN_THROWS();
}

RETVAL_NULL();
zval *id = ZEND_THIS;
dom_object *intern = Z_DOMOBJ_P(id);
dom_nnodemap_object *objmap = intern->ptr;
php_dom_named_node_map_get_item_into_zval(objmap, index, return_value);
}
/* }}} end dom_namednodemap_item */

Expand Down Expand Up @@ -254,7 +258,7 @@ PHP_METHOD(DOMNamedNodeMap, count)
}

intern = Z_DOMOBJ_P(id);
RETURN_LONG(get_namednodemap_length(intern));
RETURN_LONG(php_dom_get_namednodemap_length(intern));
}
/* }}} end dom_namednodemap_count */

Expand Down
50 changes: 23 additions & 27 deletions ext/dom/nodelist.c
View file
Expand Up @@ -31,7 +31,7 @@
* Since:
*/

static int get_nodelist_length(dom_object *obj)
int php_dom_get_nodelist_length(dom_object *obj)
{
dom_nnodemap_object *objmap = (dom_nnodemap_object *) obj->ptr;
if (!objmap) {
Expand Down Expand Up @@ -82,7 +82,7 @@ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#ID-20
*/
int dom_nodelist_length_read(dom_object *obj, zval *retval)
{
ZVAL_LONG(retval, get_nodelist_length(obj));
ZVAL_LONG(retval, php_dom_get_nodelist_length(obj));
return SUCCESS;
}

Expand All @@ -99,36 +99,15 @@ PHP_METHOD(DOMNodeList, count)
}

intern = Z_DOMOBJ_P(id);
RETURN_LONG(get_nodelist_length(intern));
RETURN_LONG(php_dom_get_nodelist_length(intern));
}
/* }}} end dom_nodelist_count */

/* }}} */

/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#ID-844377136
Since:
*/
PHP_METHOD(DOMNodeList, item)
void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value)
{
zval *id;
zend_long index;
int ret;
dom_object *intern;
xmlNodePtr itemnode = NULL;

dom_nnodemap_object *objmap;
xmlNodePtr nodep, curnode;
int count = 0;

id = ZEND_THIS;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
RETURN_THROWS();
}

if (index >= 0) {
intern = Z_DOMOBJ_P(id);

objmap = (dom_nnodemap_object *)intern->ptr;
if (objmap != NULL) {
if (objmap->ht) {
if (objmap->nodetype == XML_ENTITY_NODE) {
Expand All @@ -145,10 +124,11 @@ PHP_METHOD(DOMNodeList, item)
return;
}
} else if (objmap->baseobj) {
nodep = dom_object_get_node(objmap->baseobj);
xmlNodePtr nodep = dom_object_get_node(objmap->baseobj);
if (nodep) {
int count = 0;
if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
curnode = nodep->children;
xmlNodePtr curnode = nodep->children;
while (count < index && curnode != NULL) {
count++;
curnode = curnode->next;
Expand All @@ -175,6 +155,22 @@ PHP_METHOD(DOMNodeList, item)

RETVAL_NULL();
}

/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#ID-844377136
Since:
*/
PHP_METHOD(DOMNodeList, item)
{
zend_long index;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
RETURN_THROWS();
}

zval *id = ZEND_THIS;
dom_object *intern = Z_DOMOBJ_P(id);
dom_nnodemap_object *objmap = intern->ptr;
php_dom_nodelist_get_item_into_zval(objmap, index, return_value);
}
/* }}} end dom_nodelist_item */

ZEND_METHOD(DOMNodeList, getIterator)
Expand Down

0 comments on commit 9f7d888

Please sign in to comment.