Fix phpGH-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 phpGH-16467.

Author: ndossche

NEWS

@@ -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

ext/dom/element.c

@@ -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();
 	}
 

ext/dom/php_dom.c

@@ -1473,23 +1473,19 @@ 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 {
 			mapptr->local = xmlCharStrndup(local, len);
 			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);
-		}
+		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 {

ext/dom/tests/gh16465.phpt

@@ -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