php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81370 Possible divide by zero bug in string.c
Submitted: 2021-08-18 03:40 UTC Modified: 2021-08-19 12:34 UTC
From: yguoaz at gmail dot com Assigned: cmb (profile)
Status: Not a bug Package: Strings related
PHP Version: master-Git-2021-08-18 (Git) OS: Linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: yguoaz at gmail dot com
New email:
PHP Version: OS:

 

 [2021-08-18 03:40 UTC] 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 


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-08-18 04:04 UTC] stas@php.net
-Type: Security +Type: Bug
 [2021-08-18 08:19 UTC] 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 08:36 UTC] 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 09:48 UTC] 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-19 11:44 UTC] cmb@php.net
-Status: Open +Status: Feedback -Assigned To: +Assigned To: cmb
 [2021-08-19 11:44 UTC] cmb@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.

That is correct.  Or can you prove that this is wrong?  A single
example would be sufficient.
 [2021-08-19 11:44 UTC] cmb@php.net
-Package: *General Issues +Package: Strings related
 [2021-08-19 12:21 UTC] yguoaz at gmail dot com
-Status: Feedback +Status: Assigned
 [2021-08-19 12:21 UTC] yguoaz at gmail dot com
This should be correct. Thanks for your clarification.
 [2021-08-19 12:34 UTC] cmb@php.net
-Status: Assigned +Status: Not a bug
 [2021-08-19 12:34 UTC] cmb@php.net
Fine.  Closing then.
 
PHP Copyright © 2001-2023 The PHP Group
All rights reserved.
Last updated: Thu Feb 09 13:03:40 2023 UTC