Bug #81370 [Opn->Fbk]: Possible divide by zero bug in string.c

Edit report at https://bugs.php.net/bug.php?id=81370&edit=1 ID: 81370 Updated by: cmb@php.net Reported by: yguoaz at gmail dot com Summary: Possible divide by zero bug in string.c -Status: Open +Status: Feedback Type: Bug Package: *General Issues Operating System: Linux PHP Version: master-Git-2021-08-18 (Git) -Assigned To: +Assigned To: cmb Block user comment: N Private report: N New Comment: > chk starts as strlen(text) and is decreased by at most 1 on each > loop iteration. There are strlen(text) loop iterations. So chk > will not reach 0 within the loop. That is correct. Or can you prove that this is wrong? A single example would be sufficient. Previous Comments: ------------------------------------------------------------------------ [2021-08-18 09:48:55] nikic@php.net chk starts as strlen(text) and is decreased by at most 1 on each loop iteration. There are strlen(text) loop iterations. So chk will not reach 0 within the loop. ------------------------------------------------------------------------ [2021-08-18 08:36:42] yguoaz at gmail dot com chk is also decreased inside the loop. So I think it has a chance to equal the zero value. Do you mean ZSTR_LEN(text)and linelength must be equal? ------------------------------------------------------------------------ [2021-08-18 08:19:03] nikic@php.net This also requires chk==0, which as far as I can see can't occur for the linelength==0 case. text being an empty string is handled early. Then chk is set to the length of text and the loop also goes over the length of text. The allocation management in this function looks pretty wild though, it might make sense to rewrite it to use smart_str. ------------------------------------------------------------------------ [2021-08-18 03:40:40] yguoaz at gmail dot com Description: ------------ In the file ext/standard/string.c, the PHP_FUNCTION wordwrap has the following code: PHP_FUNCTION(wordwrap) { zend_long linelength = 75; ZEND_PARSE_PARAMETERS_START(1, 4) ... Z_PARAM_LONG(linelength) Z_PARAM_STRING(breakchar, breakchar_len) Z_PARAM_BOOL(docut) ZEND_PARSE_PARAMETERS_END(); ... if (linelength == 0 && docut) { RETURN_THROWS(); } if (breakchar_len == 1 && !docut) { ... } else { ... for (current = 0; current < (zend_long)ZSTR_LEN(text); current++) { if (chk == 0) { alloced += (size_t) (((ZSTR_LEN(text) - current + 1)/linelength + 1) * breakchar_len) + 1; newtext = zend_string_extend(newtext, alloced, 0); chk = (size_t) ((ZSTR_LEN(text) - current)/linelength) + 1; } ... } } } When the parameters satisfy linelength == 0 && docut == 0 && breakchar_len > 1, the variable linelength may be used as a divisor in the for loop, leading to a divide by zero bug. Here is the link to the related code location: https://github.com/php/php-src/blob/e86a0a905dd091a82bea2ff56845297171ea7249/ext/standard/string.c#L954 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=81370&edit=1