-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix #70359 and #78577 #11402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #70359 and #78577 #11402
Changes from 5 commits
6932c76
f7811b0
a7c5a9d
c175b91
413834e
53c767e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,26 @@ zend_object *dom_objects_new(zend_class_entry *class_type) | |
} | ||
/* }}} */ | ||
|
||
static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type) | ||
{ | ||
ZEND_ASSERT(class_type == dom_namespace_node_class_entry); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this be an issue if someone sets a custom class for namespace nodes? Otherwise, just asserting it is an instance of the namespace node class should be fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now it's not possible to register a custom class for DOMNameSpaceNode, likely because of an oversight. |
||
dom_object_namespace_node *intern = zend_object_alloc(sizeof(dom_object_namespace_node), dom_namespace_node_class_entry); | ||
dom_objects_set_class_ex(dom_namespace_node_class_entry, &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 +1580,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 DOM_RET_OBJ. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, should've said php_dom_create_object() in the comment instead of DOM_RET_OBJ... But CI is running now so I'll fix it in the merge.. |
||
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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
--TEST-- | ||
Bug #70359 (print_r() on DOMAttr causes Segfault in php_libxml_node_free_list()) | ||
--EXTENSIONS-- | ||
dom | ||
--FILE-- | ||
<?php | ||
|
||
echo "-- Test without parent --\n"; | ||
|
||
$dom = new DOMDocument(); | ||
$dom->loadXML(<<<XML | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:xsi="fooooooooooooooooooooo"/> | ||
XML); | ||
$spaceNode = $dom->documentElement->getAttributeNode('xmlns'); | ||
print_r($spaceNode); | ||
|
||
echo "-- Test with parent and non-ns attribute --\n"; | ||
|
||
$dom = new DOMDocument(); | ||
$dom->loadXML(<<<XML | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"> | ||
<url xmlns:xsi="fooooooooooooooooooooo" myattrib="bar"/> | ||
</urlset> | ||
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 | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"> | ||
<url xmlns:xsi="fooooooooooooooooooooo" myattrib="bar"/> | ||
</urlset> | ||
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" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
--TEST-- | ||
Bug #78577 (Crash in DOMNameSpace debug info handlers) | ||
--EXTENSIONS-- | ||
dom | ||
--FILE-- | ||
<?php | ||
|
||
$doc = new DOMDocument; | ||
$doc->loadXML('<foo xmlns="http://php.net/test" xmlns:foo="urn:foo" />'); | ||
|
||
$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)" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
--TEST-- | ||
DOMXPath::query() can return DOMNodeList with DOMNameSpaceNode items - advanced variation | ||
--EXTENSIONS-- | ||
dom | ||
--FILE-- | ||
<?php | ||
|
||
$dom = new DOMDocument(); | ||
$dom->loadXML(<<<'XML' | ||
<root xmlns:foo="http://example.com/foo" xmlns:bar="http://example.com/bar"> | ||
<child xmlns:baz="http://example.com/baz">Hello PHP!</child> | ||
</root> | ||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit you probably want to remove the trailing
/* {{{ */
comment