diff --git a/ext/dom/element.c b/ext/dom/element.c index 78113d72776b..f84caa629cc6 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -150,6 +150,7 @@ int dom_element_schema_type_info_read(dom_object *obj, zval *retval) /* }}} */ +/* Note: the object returned is not necessarily a node, but can be an attribute or a namespace declaration. */ static xmlNodePtr dom_get_dom1_attribute(xmlNodePtr elem, xmlChar *name) /* {{{ */ { int len; @@ -376,25 +377,13 @@ PHP_METHOD(DOMElement, getAttributeNode) } if (attrp->type == XML_NAMESPACE_DECL) { - xmlNsPtr curns; - xmlNodePtr nsparent; - - nsparent = attrp->_private; - curns = xmlNewNs(NULL, attrp->name, NULL); - if (attrp->children) { - curns->prefix = xmlStrdup((xmlChar *) attrp->children); - } - if (attrp->children) { - attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *) attrp->children, attrp->name); - } else { - attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *)"xmlns", attrp->name); - } - attrp->type = XML_NAMESPACE_DECL; - attrp->parent = nsparent; - attrp->ns = curns; + xmlNsPtr original = (xmlNsPtr) attrp; + /* Keep parent alive, because we're a fake child. */ + GC_ADDREF(&intern->std); + (void) php_dom_create_fake_namespace_decl(nodep, original, return_value, intern); + } else { + DOM_RET_OBJ((xmlNodePtr) attrp, &ret, intern); } - - DOM_RET_OBJ((xmlNodePtr) attrp, &ret, intern); } /* }}} end dom_element_get_attribute_node */ diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index df20093221f1..9e0bb1f3d1d0 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -61,6 +61,7 @@ PHP_DOM_EXPORT zend_class_entry *dom_namespace_node_class_entry; zend_object_handlers dom_object_handlers; zend_object_handlers dom_nnodemap_object_handlers; +zend_object_handlers dom_object_namespace_node_handlers; #ifdef LIBXML_XPATH_ENABLED zend_object_handlers dom_xpath_object_handlers; #endif @@ -86,6 +87,9 @@ static HashTable dom_xpath_prop_handlers; #endif /* }}} */ +static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type); +static void dom_object_namespace_node_free_storage(zend_object *object); + typedef int (*dom_read_t)(dom_object *obj, zval *retval); typedef int (*dom_write_t)(dom_object *obj, zval *newval); @@ -570,6 +574,10 @@ PHP_MINIT_FUNCTION(dom) dom_nnodemap_object_handlers.read_dimension = dom_nodelist_read_dimension; dom_nnodemap_object_handlers.has_dimension = dom_nodelist_has_dimension; + memcpy(&dom_object_namespace_node_handlers, &dom_object_handlers, sizeof(zend_object_handlers)); + dom_object_namespace_node_handlers.offset = XtOffsetOf(dom_object_namespace_node, dom.std); + dom_object_namespace_node_handlers.free_obj = dom_object_namespace_node_free_storage; + zend_hash_init(&classes, 0, NULL, NULL, 1); dom_domexception_class_entry = register_class_DOMException(zend_ce_exception); @@ -604,7 +612,7 @@ PHP_MINIT_FUNCTION(dom) zend_hash_add_ptr(&classes, dom_node_class_entry->name, &dom_node_prop_handlers); dom_namespace_node_class_entry = register_class_DOMNameSpaceNode(); - dom_namespace_node_class_entry->create_object = dom_objects_new; + dom_namespace_node_class_entry->create_object = dom_objects_namespace_node_new; zend_hash_init(&dom_namespace_node_prop_handlers, 0, NULL, dom_dtor_prop_handler, 1); dom_register_prop_handler(&dom_namespace_node_prop_handlers, "nodeName", sizeof("nodeName")-1, dom_node_node_name_read, NULL); @@ -1001,10 +1009,8 @@ void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xml } /* }}} */ -static dom_object* dom_objects_set_class(zend_class_entry *class_type) /* {{{ */ +static void dom_objects_set_class_ex(zend_class_entry *class_type, dom_object *intern) { - dom_object *intern = zend_object_alloc(sizeof(dom_object), class_type); - zend_class_entry *base_class = class_type; while ((base_class->type != ZEND_INTERNAL_CLASS || base_class->info.internal.module->module_number != dom_module_entry.module_number) && base_class->parent != NULL) { base_class = base_class->parent; @@ -1014,10 +1020,14 @@ static dom_object* dom_objects_set_class(zend_class_entry *class_type) /* {{{ */ zend_object_std_init(&intern->std, class_type); object_properties_init(&intern->std, class_type); +} +static dom_object* dom_objects_set_class(zend_class_entry *class_type) +{ + dom_object *intern = zend_object_alloc(sizeof(dom_object), class_type); + dom_objects_set_class_ex(class_type, intern); return intern; } -/* }}} */ /* {{{ dom_objects_new */ zend_object *dom_objects_new(zend_class_entry *class_type) @@ -1028,6 +1038,25 @@ zend_object *dom_objects_new(zend_class_entry *class_type) } /* }}} */ +static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type) +{ + dom_object_namespace_node *intern = zend_object_alloc(sizeof(dom_object_namespace_node), class_type); + dom_objects_set_class_ex(class_type, &intern->dom); + intern->dom.std.handlers = &dom_object_namespace_node_handlers; + return &intern->dom.std; +} + +static void dom_object_namespace_node_free_storage(zend_object *object) +{ + dom_object_namespace_node *intern = php_dom_namespace_node_obj_from_obj(object); + if (intern->parent_intern != NULL) { + zval tmp; + ZVAL_OBJ(&tmp, &intern->parent_intern->std); + zval_ptr_dtor(&tmp); + } + dom_objects_free_storage(object); +} + #ifdef LIBXML_XPATH_ENABLED /* {{{ zend_object dom_xpath_objects_new(zend_class_entry *class_type) */ zend_object *dom_xpath_objects_new(zend_class_entry *class_type) @@ -1550,6 +1579,28 @@ xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName) { } /* }}} end dom_get_nsdecl */ +/* Note: Assumes the additional lifetime was already added in the caller. */ +xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern) +{ + xmlNodePtr attrp; + xmlNsPtr curns = xmlNewNs(NULL, original->href, NULL); + if (original->prefix) { + curns->prefix = xmlStrdup(original->prefix); + attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *) original->prefix, original->href); + } else { + attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *)"xmlns", original->href); + } + attrp->type = XML_NAMESPACE_DECL; + attrp->parent = nodep; + attrp->ns = curns; + + php_dom_create_object(attrp, return_value, parent_intern); + /* This object must exist, because we just created an object for it via php_dom_create_object(). */ + dom_object *obj = ((php_libxml_node_ptr *)attrp->_private)->_private; + php_dom_namespace_node_obj_from_obj(&obj->std)->parent_intern = parent_intern; + return attrp; +} + static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int type, zval *rv) /* {{{ */ { zval offset_copy; diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index ac23d1fc25bb..6ed382b6f84a 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -93,6 +93,18 @@ typedef struct { HashPosition pos; } php_dom_iterator; +typedef struct { + /* This may be a fake object that isn't actually in the children list of the parent. + * This is because some namespace declaration nodes aren't stored on the parent in libxml2, so we have to fake it. + * We could use a zval for this, but since this is always going to be an object let's save space... */ + dom_object *parent_intern; + dom_object dom; +} dom_object_namespace_node; + +static inline dom_object_namespace_node *php_dom_namespace_node_obj_from_obj(zend_object *obj) { + return (dom_object_namespace_node*)((char*)(obj) - XtOffsetOf(dom_object_namespace_node, dom.std)); +} + #include "domexception.h" dom_object *dom_object_get_data(xmlNodePtr obj); @@ -126,6 +138,7 @@ xmlNode *php_dom_libxml_hash_iter(xmlHashTable *ht, int index); xmlNode *php_dom_libxml_notation_iter(xmlHashTable *ht, int index); zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref); void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce); +xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern); void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc); void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc); diff --git a/ext/dom/tests/bug70359.phpt b/ext/dom/tests/bug70359.phpt new file mode 100644 index 000000000000..b0a5ae57a323 --- /dev/null +++ b/ext/dom/tests/bug70359.phpt @@ -0,0 +1,83 @@ +--TEST-- +Bug #70359 (print_r() on DOMAttr causes Segfault in php_libxml_node_free_list()) +--EXTENSIONS-- +dom +--FILE-- +loadXML(<< + +XML); +$spaceNode = $dom->documentElement->getAttributeNode('xmlns'); +print_r($spaceNode); + +echo "-- Test with parent and non-ns attribute --\n"; + +$dom = new DOMDocument(); +$dom->loadXML(<< + + + +XML); +$spaceNode = $dom->documentElement->firstElementChild->getAttributeNode('myattrib'); +var_dump($spaceNode->nodeType); +var_dump($spaceNode->nodeValue); + +$dom->documentElement->firstElementChild->remove(); +try { + print_r($spaceNode->parentNode); +} catch (\Error $e) { + echo $e->getMessage(), "\n"; +} + +echo "-- Test with parent and ns attribute --\n"; + +$dom = new DOMDocument(); +$dom->loadXML(<< + + + +XML); +$spaceNode = $dom->documentElement->firstElementChild->getAttributeNode('xmlns:xsi'); +print_r($spaceNode); + +$dom->documentElement->firstElementChild->remove(); +var_dump($spaceNode->parentNode->nodeName); // Shouldn't crash + +?> +--EXPECT-- +-- Test without parent -- +DOMNameSpaceNode Object +( + [nodeName] => xmlns + [nodeValue] => http://www.sitemaps.org/schemas/sitemap/0.9 + [nodeType] => 18 + [prefix] => + [localName] => xmlns + [namespaceURI] => http://www.sitemaps.org/schemas/sitemap/0.9 + [ownerDocument] => (object value omitted) + [parentNode] => (object value omitted) +) +-- Test with parent and non-ns attribute -- +int(2) +string(3) "bar" +Couldn't fetch DOMAttr. Node no longer exists +-- Test with parent and ns attribute -- +DOMNameSpaceNode Object +( + [nodeName] => xmlns:xsi + [nodeValue] => fooooooooooooooooooooo + [nodeType] => 18 + [prefix] => xsi + [localName] => xsi + [namespaceURI] => fooooooooooooooooooooo + [ownerDocument] => (object value omitted) + [parentNode] => (object value omitted) +) +string(3) "url" diff --git a/ext/dom/tests/bug78577.phpt b/ext/dom/tests/bug78577.phpt new file mode 100644 index 000000000000..2631efc1e206 --- /dev/null +++ b/ext/dom/tests/bug78577.phpt @@ -0,0 +1,33 @@ +--TEST-- +Bug #78577 (Crash in DOMNameSpace debug info handlers) +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); + +$attr = $doc->documentElement->getAttributeNode('xmlns'); +var_dump($attr); + +?> +--EXPECT-- +object(DOMNameSpaceNode)#3 (8) { + ["nodeName"]=> + string(5) "xmlns" + ["nodeValue"]=> + string(19) "http://php.net/test" + ["nodeType"]=> + int(18) + ["prefix"]=> + string(0) "" + ["localName"]=> + string(5) "xmlns" + ["namespaceURI"]=> + string(19) "http://php.net/test" + ["ownerDocument"]=> + string(22) "(object value omitted)" + ["parentNode"]=> + string(22) "(object value omitted)" +} diff --git a/ext/dom/tests/xpath_domnamespacenode.phpt b/ext/dom/tests/xpath_domnamespacenode.phpt index f0bfbed10dda..97059c18e54d 100644 --- a/ext/dom/tests/xpath_domnamespacenode.phpt +++ b/ext/dom/tests/xpath_domnamespacenode.phpt @@ -17,7 +17,7 @@ var_dump($nodes->item(0)); ?> --EXPECT-- -object(DOMNameSpaceNode)#3 (8) { +object(DOMNameSpaceNode)#4 (8) { ["nodeName"]=> string(9) "xmlns:xml" ["nodeValue"]=> diff --git a/ext/dom/tests/xpath_domnamespacenode_advanced.phpt b/ext/dom/tests/xpath_domnamespacenode_advanced.phpt new file mode 100644 index 000000000000..bbc49dc54652 --- /dev/null +++ b/ext/dom/tests/xpath_domnamespacenode_advanced.phpt @@ -0,0 +1,75 @@ +--TEST-- +DOMXPath::query() can return DOMNodeList with DOMNameSpaceNode items - advanced variation +--EXTENSIONS-- +dom +--FILE-- +loadXML(<<<'XML' + + Hello PHP! + +XML); + +$xpath = new DOMXPath($dom); +$query = '//namespace::*'; + +echo "-- All namespace attributes --\n"; + +foreach ($xpath->query($query) as $attribute) { + echo $attribute->nodeName . ' = ' . $attribute->nodeValue . PHP_EOL; + var_dump($attribute->parentNode->tagName); +} + +echo "-- All namespace attributes with removal attempt --\n"; + +foreach ($xpath->query($query) as $attribute) { + echo "Before: ", $attribute->parentNode->tagName, "\n"; + // Second & third attempt should fail because it's no longer in the document + try { + $attribute->parentNode->remove(); + } catch (\DOMException $e) { + echo $e->getMessage(), "\n"; + } + // However, it should not cause a use-after-free + echo "After: ", $attribute->parentNode->tagName, "\n"; +} + +?> +--EXPECT-- +-- All namespace attributes -- +xmlns:xml = http://www.w3.org/XML/1998/namespace +string(4) "root" +xmlns:bar = http://example.com/bar +string(4) "root" +xmlns:foo = http://example.com/foo +string(4) "root" +xmlns:xml = http://www.w3.org/XML/1998/namespace +string(5) "child" +xmlns:bar = http://example.com/bar +string(5) "child" +xmlns:foo = http://example.com/foo +string(5) "child" +xmlns:baz = http://example.com/baz +string(5) "child" +-- All namespace attributes with removal attempt -- +Before: root +After: root +Before: root +Not Found Error +After: root +Before: root +Not Found Error +After: root +Before: child +After: child +Before: child +Not Found Error +After: child +Before: child +Not Found Error +After: child +Before: child +Not Found Error +After: child diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c index f546733a436d..62e11f6b99bf 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -101,24 +101,18 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, zval child; /* not sure, if we need this... it's copied from xpath.c */ if (node->type == XML_NAMESPACE_DECL) { - xmlNsPtr curns; - xmlNodePtr nsparent; - - nsparent = node->_private; - curns = xmlNewNs(NULL, node->name, NULL); - if (node->children) { - curns->prefix = xmlStrdup((xmlChar *) node->children); - } - if (node->children) { - node = xmlNewDocNode(node->doc, NULL, (xmlChar *) node->children, node->name); - } else { - node = xmlNewDocNode(node->doc, NULL, (xmlChar *) "xmlns", node->name); - } - node->type = XML_NAMESPACE_DECL; - node->parent = nsparent; - node->ns = curns; + xmlNodePtr nsparent = node->_private; + xmlNsPtr original = (xmlNsPtr) node; + + /* Make sure parent dom object exists, so we can take an extra reference. */ + zval parent_zval; /* don't destroy me, my lifetime is transfered to the fake namespace decl */ + php_dom_create_object(nsparent, &parent_zval, &intern->dom); + dom_object *parent_intern = Z_DOMOBJ_P(&parent_zval); + + node = php_dom_create_fake_namespace_decl(nsparent, original, &child, parent_intern); + } else { + php_dom_create_object(node, &child, &intern->dom); } - php_dom_create_object(node, &child, &intern->dom); add_next_index_zval(&fci.params[i], &child); } } else { @@ -421,24 +415,18 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */ zval child; if (node->type == XML_NAMESPACE_DECL) { - xmlNsPtr curns; - xmlNodePtr nsparent; + xmlNodePtr nsparent = node->_private; + xmlNsPtr original = (xmlNsPtr) node; - nsparent = node->_private; - curns = xmlNewNs(NULL, node->name, NULL); - if (node->children) { - curns->prefix = xmlStrdup((xmlChar *) node->children); - } - if (node->children) { - node = xmlNewDocNode(docp, NULL, (xmlChar *) node->children, node->name); - } else { - node = xmlNewDocNode(docp, NULL, (xmlChar *) "xmlns", node->name); - } - node->type = XML_NAMESPACE_DECL; - node->parent = nsparent; - node->ns = curns; + /* Make sure parent dom object exists, so we can take an extra reference. */ + zval parent_zval; /* don't destroy me, my lifetime is transfered to the fake namespace decl */ + php_dom_create_object(nsparent, &parent_zval, &intern->dom); + dom_object *parent_intern = Z_DOMOBJ_P(&parent_zval); + + node = php_dom_create_fake_namespace_decl(nsparent, original, &child, parent_intern); + } else { + php_dom_create_object(node, &child, &intern->dom); } - php_dom_create_object(node, &child, &intern->dom); add_next_index_zval(&retval, &child); } } else {