Fix GH-16465: Heap buffer overflow in DOMNode->getElementByTagName

If the input contains NUL bytes then the length doesn't match the actual
duplicated string's length. Note that libxml can't handle this properly
anyway so we just reject NUL bytes and too long strings.

Closes GH-16467.
This commit is contained in:
Niels Dossche 2024-10-16 20:05:41 +02:00
parent ef1c3b82ff
commit d70f3ba9a5
No known key found for this signature in database
GPG key ID: B8A8AD166DF0E2E5
4 changed files with 51 additions and 9 deletions

2
NEWS
View file

@ -26,6 +26,8 @@ PHP NEWS
. Fixed bug GH-16316 (DOMXPath breaks when not initialized properly).
(nielsdos)
. Add missing hierarchy checks to replaceChild. (nielsdos)
. Fixed bug GH-16465 (Heap buffer overflow in DOMNode->getElementByTagName).
(nielsdos)
- EXIF:
. Fixed bug GH-16409 (Segfault in exif_thumbnail when not dealing with a

View file

@ -816,7 +816,12 @@ static void dom_element_get_elements_by_tag_name(INTERNAL_FUNCTION_PARAMETERS, b
dom_object *intern, *namednode;
char *name;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &name, &name_len) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "p", &name, &name_len) == FAILURE) {
RETURN_THROWS();
}
if (name_len > INT_MAX) {
zend_argument_value_error(1, "is too long");
RETURN_THROWS();
}
@ -1239,7 +1244,17 @@ static void dom_element_get_elements_by_tag_name_ns(INTERNAL_FUNCTION_PARAMETERS
dom_object *intern, *namednode;
char *uri, *name;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s!s", &uri, &uri_len, &name, &name_len) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "p!p", &uri, &uri_len, &name, &name_len) == FAILURE) {
RETURN_THROWS();
}
if (uri_len > INT_MAX) {
zend_argument_value_error(1, "is too long");
RETURN_THROWS();
}
if (name_len > INT_MAX) {
zend_argument_value_error(2, "is too long");
RETURN_THROWS();
}

View file

@ -1473,7 +1473,7 @@ void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xml
const xmlChar* tmp;
if (local) {
int len = local_len > INT_MAX ? -1 : (int) local_len;
int len = (int) local_len;
if (doc != NULL && (tmp = xmlDictExists(doc->dict, (const xmlChar *)local, len)) != NULL) {
mapptr->local = BAD_CAST tmp;
} else {
@ -1481,15 +1481,11 @@ void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xml
mapptr->free_local = true;
}
mapptr->local_lower = BAD_CAST estrdup(local);
if (len < 0) {
zend_str_tolower((char *) mapptr->local_lower, strlen((const char *) mapptr->local_lower));
} else {
zend_str_tolower((char *) mapptr->local_lower, len);
}
}
if (ns) {
int len = ns_len > INT_MAX ? -1 : (int) ns_len;
int len = (int) ns_len;
if (doc != NULL && (tmp = xmlDictExists(doc->dict, (const xmlChar *)ns, len)) != NULL) {
mapptr->ns = BAD_CAST tmp;
} else {

View file

@ -0,0 +1,29 @@
--TEST--
GH-16465 (Heap buffer overflow in DOMNode->getElementByTagName)
--EXTENSIONS--
dom
--FILE--
<?php
$v10 = new DOMElement("a");
try {
$v10->getElementsByTagName("text\0something");
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}
try {
$v10->getElementsByTagNameNS("", "text\0something");
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}
try {
$v10->getElementsByTagNameNS("text\0something", "");
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}
?>
--EXPECT--
DOMElement::getElementsByTagName(): Argument #1 ($qualifiedName) must not contain any null bytes
DOMElement::getElementsByTagNameNS(): Argument #2 ($localName) must not contain any null bytes
DOMElement::getElementsByTagNameNS(): Argument #1 ($namespace) must not contain any null bytes