Fix #72714: _xml_startElementHandler() segmentation fault

The issue is caused by an integer overflow when the `long` passed as
XML_OPTION_SKIP_TAGSTART is assigned to `xml_parser::toffset` which is
declared as `int`. We can simply work around this issue, by clipping
resulting negative values to 0 (and raising a notice in this case), because
the reasonable range for this value is certainly catered to by positive
`int`s.

However, there still remains the issue that `xml_parser::toffset` is later
added to `char *`s, which can cause OOB reads, so we make sure that the
upper bound never exceeds the strlen(). We eschew optimizing `SKIP_TAGSTART`
wrt. to the potentially duplicate strlen() call, because that code path is
unexpected anyway.
This commit is contained in:
Christoph M. Becker 2016-08-16 20:36:33 +02:00
parent f682193609
commit 9164dc11e2
3 changed files with 52 additions and 8 deletions

View file

@ -66,6 +66,10 @@ ZEND_GET_MODULE(xml)
#endif /* COMPILE_DL_XML */
/* }}} */
#define SKIP_TAGSTART(str) ((str) + (parser->toffset > strlen(str) ? strlen(str) : + parser->toffset))
/* {{{ function prototypes */
PHP_MINIT_FUNCTION(xml);
PHP_MINFO_FUNCTION(xml);
@ -785,7 +789,7 @@ void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Ch
if (parser->startElementHandler) {
args[0] = _xml_resource_zval(parser->index);
args[1] = _xml_string_zval(((char *) tag_name) + parser->toffset);
args[1] = _xml_string_zval(SKIP_TAGSTART((char *) tag_name));
MAKE_STD_ZVAL(args[2]);
array_init(args[2]);
@ -816,9 +820,9 @@ void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Ch
array_init(tag);
array_init(atr);
_xml_add_to_info(parser,((char *) tag_name) + parser->toffset);
_xml_add_to_info(parser,SKIP_TAGSTART((char *) tag_name));
add_assoc_string(tag,"tag",((char *) tag_name) + parser->toffset,1); /* cast to avoid gcc-warning */
add_assoc_string(tag,"tag",SKIP_TAGSTART((char *) tag_name),1);
add_assoc_string(tag,"type","open",1);
add_assoc_long(tag,"level",parser->level);
@ -870,7 +874,7 @@ void _xml_endElementHandler(void *userData, const XML_Char *name)
if (parser->endElementHandler) {
args[0] = _xml_resource_zval(parser->index);
args[1] = _xml_string_zval(((char *) tag_name) + parser->toffset);
args[1] = _xml_string_zval(SKIP_TAGSTART((char *) tag_name));
if ((retval = xml_call_handler(parser, parser->endElementHandler, parser->endElementPtr, 2, args))) {
zval_ptr_dtor(&retval);
@ -887,9 +891,9 @@ void _xml_endElementHandler(void *userData, const XML_Char *name)
array_init(tag);
_xml_add_to_info(parser,((char *) tag_name) + parser->toffset);
_xml_add_to_info(parser,SKIP_TAGSTART((char *) tag_name));
add_assoc_string(tag,"tag",((char *) tag_name) + parser->toffset,1); /* cast to avoid gcc-warning */
add_assoc_string(tag,"tag",SKIP_TAGSTART((char *) tag_name),1);
add_assoc_string(tag,"type","close",1);
add_assoc_long(tag,"level",parser->level);
@ -990,9 +994,9 @@ void _xml_characterDataHandler(void *userData, const XML_Char *s, int len)
array_init(tag);
_xml_add_to_info(parser,parser->ltags[parser->level-1] + parser->toffset);
_xml_add_to_info(parser,SKIP_TAGSTART(parser->ltags[parser->level-1]));
add_assoc_string(tag,"tag",parser->ltags[parser->level-1] + parser->toffset,1);
add_assoc_string(tag,"tag",SKIP_TAGSTART(parser->ltags[parser->level-1]),1);
add_assoc_string(tag,"value",decoded_value,0);
add_assoc_string(tag,"type","cdata",1);
add_assoc_long(tag,"level",parser->level);
@ -1633,6 +1637,10 @@ PHP_FUNCTION(xml_parser_set_option)
case PHP_XML_OPTION_SKIP_TAGSTART:
convert_to_long_ex(val);
parser->toffset = Z_LVAL_PP(val);
if (parser->toffset < 0) {
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "tagstart ignored");
parser->toffset = 0;
}
break;
case PHP_XML_OPTION_SKIP_WHITE:
convert_to_long_ex(val);