UBSan mb_convert_variables() with long string

[YuanchengJiang]

Description

The following code:

<?php
xml_parse_into_struct(xml_parser_create_ns(), str_repeat("<blah>", 1000), $a);
$fusion = $a;
var_dump(mb_convert_variables("ASCII", ["UTF-8", "UTF-16"], $fusion));

Resulted in this output:

/home/phpfuzz/WorkSpace/flowfusion/php-src/ext/mbstring/mbstring.c:3351:23: runtime error: 1.91594e+19 is outside the range of representable values of type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/phpfuzz/WorkSpace/flowfusion/php-src/ext/mbstring/mbstring.c:3351:23

PHP Version

nightly

Operating System

No response

[devnexen]

can't reproduce. First XML_MAXLEVEL prevents it from the get go. Even if I update it I still do not get the overflow in mbstring. But I guess it does not kill to put a check.

[youkidearitai]

I can reproduce. seems curious.

$ sapi/cli/php -dmemory_limit=-1 gh17503.php
/home/tekimen/src/php-src/Zend/zend_ini_parser.y:377:4: runtime error: call to function php_ini_parser_cb through pointer to incorrect function type 'void (*)(struct _zval_struct *, struct _zval_struct *, struct _zval_struct *, int, void *)'
/home/tekimen/src/php-src/main/php_ini.c:184: note: php_ini_parser_cb defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/Zend/zend_ini_parser.y:377:4
/home/tekimen/src/php-src/Zend/zend_API.c:2424:4: runtime error: call to function zm_globals_ctor_date through pointer to incorrect function type 'void (*)(void *)'
/home/tekimen/src/php-src/ext/date/php_date.c:395: note: zm_globals_ctor_date defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/Zend/zend_API.c:2424:4
/home/tekimen/src/php-src/Zend/zend_variables.c:57:2: runtime error: call to function zend_string_destroy through pointer to incorrect function type 'void (*)(struct _zend_refcounted *)'
/home/tekimen/src/php-src/Zend/zend_variables.c:62: note: zend_string_destroy defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/Zend/zend_variables.c:57:2

Warning: xml_parse_into_struct(): Maximum depth exceeded - Results truncated in /home/tekimen/src/php-src/gh17503.php on line 3
/home/tekimen/src/php-src/ext/mbstring/mbstring.c:3351:23: runtime error: 1.91594e+19 is outside the range of representable values of type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/ext/mbstring/mbstring.c:3351:23
string(5) "UTF-8"
/home/tekimen/src/php-src/Zend/zend_llist.c:93:7: runtime error: call to function zend_compare_file_handles through pointer to incorrect function type 'int (*)(void *, void *)'
/home/tekimen/src/php-src/Zend/zend_stream.c:251: note: zend_compare_file_handles defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/Zend/zend_llist.c:93:7
/home/tekimen/src/php-src/Zend/zend_llist.c:94:4: runtime error: call to function zend_file_handle_dtor through pointer to incorrect function type 'void (*)(void *)'
/home/tekimen/src/php-src/Zend/zend_stream.c:214: note: zend_file_handle_dtor defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/Zend/zend_llist.c:94:4
/home/tekimen/src/php-src/Zend/zend_API.c:3341:4: runtime error: call to function zm_globals_dtor_phar through pointer to incorrect function type 'void (*)(void *)'
/home/tekimen/src/php-src/ext/phar/phar.c:3410: note: zm_globals_dtor_phar defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/tekimen/src/php-src/Zend/zend_API.c:3341:4

[devnexen]

Ah right I can only reproduce with clang :) feel free to assign to yourself.

[cmb69]

php-src/ext/mbstring/mbstring.c

Line 3351 in 675f359

array[i].demerits = (uint64_t) (array[i].demerits * array[i].multiplier);

I think we need to clamp the value instead of casting to uint64_t (might need the cast in addition, though). Possibly:

		array[i].demerits = MIN(array[i].demerits * array[i].multiplier, UINT64_MAX);

@youkidearitai, regarding the "call to function through pointer to incorrect function type" errors, see #17462. TL; DR: also build with -fno-sanitize=function.

[alexdowad]

@cmb69 is right that merely casting, rather than clamping, will cause mb_detect_encoding to make very strange guesses of input text encoding.

[alexdowad]

@cmb69 is right that merely casting, rather than clamping, will cause mb_detect_encoding to make very strange guesses of input text encoding.

...Which, by the way, is my fault. 😄 Thanks to @YuanchengJiang for catching my mistake.