Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory_to_bytes() INI parsing issue (32bit) #556

Closed
crispy-computing-machine opened this issue Jul 13, 2021 · 3 comments · Fixed by #561
Closed

memory_to_bytes() INI parsing issue (32bit) #556

crispy-computing-machine opened this issue Jul 13, 2021 · 3 comments · Fixed by #561
Labels

Comments

@crispy-computing-machine
Copy link

crispy-computing-machine commented Jul 13, 2021

Bug report

Question Answer
Box version Box version 3.13.0@275b091
PHP version PHP 7.4.18 (cli) (built: Apr 27 2021 17:19:39) ( ZTS Visual C++ 2017 x86 )
Platform with version Windows 10
Github Repo NA

To replicate:
Run box.phar compile with 2G as PHP memory_limit INI setting.

Error:

PHP Fatal error: Uncaught TypeError: Return value of _HumbugBox113887eee2b6\KevinGH\Box\memory_to_bytes() must be of the type int, float returned in phar://box.phar/src/functions.php:81
Stack trace:
0 phar://box.phar/src/Console/Php/PhpSettingsHandler.php(62): _HumbugBox113887eee2b6\KevinGH\Box\memory_to_bytes('2G')
1 phar://box.phar/src/Console/Php/PhpSettingsHandler.php(33): _HumbugBox113887eee2b6\KevinGH\Box\Console\Php\PhpSettingsHandler->bumpMemoryLimit()
2 phar://box.phar/src/functions.php(124): _HumbugBox113887eee2b6\KevinGH\Box\Console\Php\PhpSettingsHandler->check()
3 phar://box.phar/src/Console/Command/Compile.php(105): _HumbugBox113887eee2b6\KevinGH\Box\check_php_settings(Object(_HumbugBox113887eee2b6\Kevin in phar://box.phar/src/functions.php on line 81

@theofidry
Copy link
Member

Hi, thanks for the bug report; is it something you can manage to reproduce consistently?

The problematic function is:

function memory_to_bytes(string $value): int
{
    $unit = strtolower($value[strlen($value) - 1]);

    $bytes = (int) $value;

    switch ($unit) {
        case 'g':
            $bytes *= 1024;
        // no break (cumulative multiplier)
        case 'm':
            $bytes *= 1024;
        // no break (cumulative multiplier)
        case 'k':
            $bytes *= 1024;
    }

    return $bytes;
}

but I hardly see how $bytes can be a float there: it is an int and is only being manipulated with ints...

@crispy-computing-machine
Copy link
Author

After a bit of reading it looks like an Integer overflow as the number goes over PHP_INT_MAX on 32bit PHP installs.

Looking at the values as they are multiplied in the function as it gets to the kilobytes one the value is interpreted as a float.

int(2048)
int(2097152)
float(2147483648)

Suggestion:

    /**
     * @param string $value
     * @return int
     */
    function memory_to_bytes(string $value): int
    {
        $unit = strtolower($value[strlen($value) - 1]);
        $bytes = substr($value, 0, -1);

        switch ($unit) {
            case 'g':
                $bytes *= 1024;
            // no break (cumulative multiplier)
            case 'm':
                $bytes *= 1024;
            // no break (cumulative multiplier)
            case 'k':
                $bytes *= 1024;
        }
        
        // If 32bit limit memory to max 
        if(PHP_INT_SIZE === 4){
            if ($bytes > PHP_INT_MAX) {
                $bytes = PHP_INT_MAX;
            }
        }

        return $bytes;
    }

@crispy-computing-machine crispy-computing-machine changed the title memory_to_bytes() INI parsing issue memory_to_bytes() INI parsing issue (32bit) Jul 14, 2021
@theofidry
Copy link
Member

Let's keep this open, I would be happy to implement your suggestion as a fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants