Skip to content

Commit 20ac42e

Browse files
committed
Fix memory leak when setting an invalid DOMDocument encoding
Because the failure path did not release the string, there was a memory leak. As the only valid types for this function are IS_NULL and IS_STRING, we and IS_NULL is always rejected in practice, solve the issue by not using a function that increments the refcount in the first place. Closes GH-12002.
1 parent fc8d5c7 commit 20ac42e

File tree

3 files changed

+53
-7
lines changed

3 files changed

+53
-7
lines changed

NEWS

+3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ PHP NEWS
55
- Core:
66
. Fixed bug GH-11937 (Constant ASTs containing objects). (ilutov)
77

8+
- DOM:
9+
. Fix memory leak when setting an invalid DOMDocument encoding. (nielsdos)
10+
811
- Iconv:
912
. Fixed build for NetBSD which still uses the old iconv signature.
1013
(David Carlier)

ext/dom/document.c

+12-7
Original file line numberDiff line numberDiff line change
@@ -139,19 +139,22 @@ int dom_document_encoding_read(dom_object *obj, zval *retval)
139139
zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
140140
{
141141
xmlDoc *docp = (xmlDocPtr) dom_object_get_node(obj);
142-
zend_string *str;
143142
xmlCharEncodingHandlerPtr handler;
144143

145144
if (docp == NULL) {
146145
php_dom_throw_error(INVALID_STATE_ERR, 1);
147146
return FAILURE;
148147
}
149148

150-
str = zval_try_get_string(newval);
151-
if (UNEXPECTED(!str)) {
152-
return FAILURE;
149+
/* Typed property, can only be IS_STRING or IS_NULL. */
150+
ZEND_ASSERT(Z_TYPE_P(newval) == IS_STRING || Z_TYPE_P(newval) == IS_NULL);
151+
152+
if (Z_TYPE_P(newval) == IS_NULL) {
153+
goto invalid_encoding;
153154
}
154155

156+
zend_string *str = Z_STR_P(newval);
157+
155158
handler = xmlFindCharEncodingHandler(ZSTR_VAL(str));
156159

157160
if (handler != NULL) {
@@ -161,12 +164,14 @@ zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
161164
}
162165
docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str));
163166
} else {
164-
zend_value_error("Invalid document encoding");
165-
return FAILURE;
167+
goto invalid_encoding;
166168
}
167169

168-
zend_string_release_ex(str, 0);
169170
return SUCCESS;
171+
172+
invalid_encoding:
173+
zend_value_error("Invalid document encoding");
174+
return FAILURE;
170175
}
171176

172177
/* }}} */

ext/dom/tests/gh12002.phpt

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-12002 (DOMDocument::encoding memory leak with invalid encoding)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function make_nonconst(string $x) {
9+
// Defeat SCCP, even with inlining
10+
return str_repeat($x, random_int(1, 1));
11+
}
12+
13+
$dom = new DOMDocument();
14+
$dom->encoding = make_nonconst('utf-8');
15+
var_dump($dom->encoding);
16+
try {
17+
$dom->encoding = make_nonconst('foobar');
18+
} catch (ValueError $e) {
19+
echo $e->getMessage(), "\n";
20+
}
21+
var_dump($dom->encoding);
22+
$dom->encoding = make_nonconst('utf-16le');
23+
var_dump($dom->encoding);
24+
try {
25+
$dom->encoding = NULL;
26+
} catch (ValueError $e) {
27+
echo $e->getMessage(), "\n";
28+
}
29+
var_dump($dom->encoding);
30+
31+
?>
32+
--EXPECT--
33+
string(5) "utf-8"
34+
Invalid document encoding
35+
string(5) "utf-8"
36+
string(8) "utf-16le"
37+
Invalid document encoding
38+
string(8) "utf-16le"

0 commit comments

Comments
 (0)