Skip to content

Commit 9cb23a3

Browse files
committed
Fix GH-15654: Signed integer overflow in ext/dom/nodelist.c
There's implicit truncation casts from zend_long to int which cause issues because checks are done against the zend_longs. Since the iterator infrastructure uses zend_longs, just convert everything to zend_long. Closes GH-15669.
1 parent 8ad7d8f commit 9cb23a3

File tree

6 files changed

+43
-9
lines changed

6 files changed

+43
-9
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ PHP NEWS
2020
- DOM:
2121
. Fixed bug GH-15551 (Segmentation fault (access null pointer) in
2222
ext/dom/xml_common.h). (nielsdos)
23+
. Fixed bug GH-15654 (Signed integer overflow in ext/dom/nodelist.c).
24+
(nielsdos)
2325

2426
- MySQLnd:
2527
. Fixed bug GH-15432 (Heap corruption when querying a vector). (cmb,

ext/dom/dom_iterators.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */
179179
dom_object *intern;
180180
dom_object *nnmap;
181181
dom_nnodemap_object *objmap;
182-
int previndex;
182+
zend_long previndex;
183183
HashTable *nodeht;
184184
zval *entry;
185185
bool do_curobj_undef = 1;
@@ -269,7 +269,7 @@ zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, i
269269
dom_object *intern;
270270
dom_nnodemap_object *objmap;
271271
xmlNodePtr curnode=NULL;
272-
int curindex = 0;
272+
zend_long curindex = 0;
273273
HashTable *nodeht;
274274
zval *entry;
275275
php_dom_iterator *iterator;

ext/dom/nodelist.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ int php_dom_get_nodelist_length(dom_object *obj)
9191
reset_objmap_cache(objmap);
9292
}
9393

94-
int count = 0;
94+
zend_long count = 0;
9595
if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
9696
xmlNodePtr curnode = dom_nodelist_iter_start_first_child(nodep);
9797
if (curnode) {
@@ -109,7 +109,7 @@ int php_dom_get_nodelist_length(dom_object *obj)
109109
nodep = nodep->children;
110110
}
111111
dom_get_elements_by_tag_name_ns_raw(
112-
basep, nodep, (char *) objmap->ns, (char *) objmap->local, &count, INT_MAX - 1 /* because of <= */);
112+
basep, nodep, (char *) objmap->ns, (char *) objmap->local, &count, ZEND_LONG_MAX - 1 /* because of <= */);
113113
}
114114

115115
objmap->cached_length = count;
@@ -174,7 +174,7 @@ void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long
174174
* TODO: in the future we could extend the logic of the node list such that backwards searches
175175
* are also possible. */
176176
bool restart = true;
177-
int relative_index = index;
177+
zend_long relative_index = index;
178178
if (index >= objmap->cached_obj_index && objmap->cached_obj && !php_dom_is_cache_tag_stale_from_node(&objmap->cache_tag, nodep)) {
179179
xmlNodePtr cached_obj_xml_node = dom_object_get_node(objmap->cached_obj);
180180

@@ -192,7 +192,7 @@ void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long
192192
nodep = cached_obj_xml_node;
193193
}
194194
}
195-
int count = 0;
195+
zend_long count = 0;
196196
if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
197197
if (restart) {
198198
nodep = dom_nodelist_iter_start_first_child(nodep);

ext/dom/php_dom.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,7 @@ bool dom_has_feature(zend_string *feature, zend_string *version)
13341334
}
13351335
/* }}} end dom_has_feature */
13361336

1337-
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, char *ns, char *local, int *cur, int index) /* {{{ */
1337+
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, char *ns, char *local, zend_long *cur, zend_long index) /* {{{ */
13381338
{
13391339
/* Can happen with detached document */
13401340
if (UNEXPECTED(nodep == NULL)) {

ext/dom/php_dom.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ typedef struct _dom_nnodemap_object {
8888
xmlChar *ns;
8989
php_libxml_cache_tag cache_tag;
9090
dom_object *cached_obj;
91-
int cached_obj_index;
91+
zend_long cached_obj_index;
9292
bool free_local : 1;
9393
bool free_ns : 1;
9494
} dom_nnodemap_object;
@@ -133,7 +133,7 @@ void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep);
133133
void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last);
134134
xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName);
135135
void dom_normalize (xmlNodePtr nodep);
136-
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, char *ns, char *local, int *cur, int index);
136+
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr basep, xmlNodePtr nodep, char *ns, char *local, zend_long *cur, zend_long index);
137137
void php_dom_create_implementation(zval *retval);
138138
int dom_hierarchy(xmlNodePtr parent, xmlNodePtr child);
139139
bool dom_has_feature(zend_string *feature, zend_string *version);

ext/dom/tests/gh15654.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-15654 (Signed integer overflow in ext/dom/nodelist.c)
3+
--EXTENSIONS--
4+
dom
5+
--SKIPIF--
6+
<?php
7+
if (PHP_INT_SIZE != 8) die('skip 64-bit only');
8+
?>
9+
--FILE--
10+
<?php
11+
define("MAX_64Bit", 9223372036854775807);
12+
define("MAX_32Bit", 2147483647);
13+
define("MIN_64Bit", -9223372036854775807 - 1);
14+
define("MIN_32Bit", -2147483647 - 1);
15+
$longVals = array(
16+
0, MAX_64Bit, MIN_64Bit, MAX_32Bit, MIN_32Bit, MAX_64Bit - MAX_32Bit, MIN_64Bit - MIN_32Bit,
17+
);
18+
$dom = new DOMDocument;
19+
$dom->loadXML('<root><a/><b/><c/></root>');
20+
$children = $dom->documentElement->childNodes;
21+
foreach ($longVals as $value) {
22+
var_dump($children[$value]?->nodeName);
23+
}
24+
?>
25+
--EXPECT--
26+
string(1) "a"
27+
NULL
28+
NULL
29+
NULL
30+
NULL
31+
NULL
32+
NULL

0 commit comments

Comments
 (0)