Skip to content

Commit d5a57af

Browse files
committed
ext/xml: Use zend_string_safe_realloc() for cdata concatenation.
The previous code computed `Z_STRLEN_P(myval) + ZSTR_LEN(decoded_value)` as a plain `size_t` addition before passing the result to zend_string_extend(), which can wrap on 32-bit and lead to a heap overflow in the following strncpy(). Switch to zend_string_safe_realloc() so the size computation is bounds-checked. close GH-22056
1 parent 10704f0 commit d5a57af

1 file changed

Lines changed: 6 additions & 8 deletions

File tree

ext/xml/xml.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -828,16 +828,15 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
828828
zval *myval;
829829
/* check if the current tag already has a value - if yes append to that! */
830830
if ((myval = zend_hash_find(Z_ARRVAL_P(ctag), ZSTR_KNOWN(ZEND_STR_VALUE))) && Z_TYPE_P(myval) == IS_STRING) {
831-
size_t newlen = Z_STRLEN_P(myval) + ZSTR_LEN(decoded_value);
832-
Z_STR_P(myval) = zend_string_extend(Z_STR_P(myval), newlen, 0);
831+
Z_STR_P(myval) = zend_string_safe_realloc(Z_STR_P(myval), 1, Z_STRLEN_P(myval), ZSTR_LEN(decoded_value), false);
833832
strncpy(Z_STRVAL_P(myval) + Z_STRLEN_P(myval) - ZSTR_LEN(decoded_value),
834833
ZSTR_VAL(decoded_value), ZSTR_LEN(decoded_value) + 1);
835-
zend_string_release_ex(decoded_value, 0);
834+
zend_string_release_ex(decoded_value, false);
836835
} else {
837836
if (doprint || (! parser->skipwhite)) {
838837
add_assoc_str(ctag, "value", decoded_value);
839838
} else {
840-
zend_string_release_ex(decoded_value, 0);
839+
zend_string_release_ex(decoded_value, false);
841840
}
842841
}
843842
} else {
@@ -855,11 +854,10 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
855854
if (EXPECTED(Z_TYPE_P(mytype) == IS_STRING) && zend_string_equals_literal(Z_STR_P(mytype), "cdata")) {
856855
SEPARATE_ARRAY(curtag);
857856
if ((myval = zend_hash_find(Z_ARRVAL_P(curtag), ZSTR_KNOWN(ZEND_STR_VALUE)))) {
858-
size_t newlen = Z_STRLEN_P(myval) + ZSTR_LEN(decoded_value);
859-
Z_STR_P(myval) = zend_string_extend(Z_STR_P(myval), newlen, 0);
857+
Z_STR_P(myval) = zend_string_safe_realloc(Z_STR_P(myval), 1, Z_STRLEN_P(myval), ZSTR_LEN(decoded_value), false);
860858
strncpy(Z_STRVAL_P(myval) + Z_STRLEN_P(myval) - ZSTR_LEN(decoded_value),
861859
ZSTR_VAL(decoded_value), ZSTR_LEN(decoded_value) + 1);
862-
zend_string_release_ex(decoded_value, 0);
860+
zend_string_release_ex(decoded_value, false);
863861
return;
864862
}
865863
}
@@ -877,7 +875,7 @@ void xml_characterDataHandler(void *userData, const XML_Char *s, int len)
877875
} else if (parser->level == (XML_MAXLEVEL + 1)) {
878876
php_error_docref(NULL, E_WARNING, "Maximum depth exceeded - Results truncated");
879877
} else {
880-
zend_string_release_ex(decoded_value, 0);
878+
zend_string_release_ex(decoded_value, false);
881879
}
882880
}
883881
}

0 commit comments

Comments
 (0)