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

Floating point numbers cause a memory error #47935

Closed
TheAndrey opened this issue Aug 2, 2023 · 5 comments
Closed

Floating point numbers cause a memory error #47935

TheAndrey opened this issue Aug 2, 2023 · 5 comments

Comments

@TheAndrey
Copy link

TheAndrey commented Aug 2, 2023

Laravel Version

10.16.1

PHP Version

8.1.21

Database Driver & Version

Ver 15.1 Distrib 10.5.11-MariaDB, for Win64 (AMD64)

Description

I made a form in which present a field for entering the amount. During testing, it turned out that entering very large values as 10.9e-10000000000 leads to the error of exhausting the PHP memory limit.

Symfony\\Component\\ErrorHandler\\Error\\FatalError: Allowed memory size of 1073741824 bytes exhausted (tried to allocate 10000000033 bytes) at vendor/brick/math/src/BigDecimal.php:737

I did the validation as follows:

$request->validate([
    'amount' => ['required', 'max:99999', 'numeric'],
]);

I tried changing the rule to decimal:0,2 and integer and it doesn't help. The restriction on the length of the string using the max: rule stops working when there are rules for validating numbers in the chain.

What should I do to prevent validator from trying to convert a string to a large number? 😞
My application is supposed to work with small numbers that fits into the size of a regular float. I don't need BigDecimal at all.

I have never had such problems with pure PHP...

$ php -dmemory_limit=2M -r "var_dump(floatval('10.9e-10000000000'));"

Steps To Reproduce

  1. Create any validator that has a rule associated with validating numeric values.
  2. There should be more/less checks in the chain of rules, such as: min, max, between.
  3. Send a request with a very large number (e.g. 10.9e-10000000000).
  4. You should get a PHP memory error or the application will hang.
@timacdonald
Copy link
Member

timacdonald commented Aug 3, 2023

@TheAndrey brick math is required because PHP cannot natively handle large float values.

Screen Shot 2023-08-03 at 9 56 33 am Screen Shot 2023-08-03 at 9 56 52 am Screen Shot 2023-08-03 at 10 09 42 am

With Brick math in place, we can perform validation against these larger numbers that PHP cannot natively handle.

Screen Shot 2023-08-03 at 10 02 33 am

I would recommend using something like a regex validation rule along with bail to ensure that the number is a reasonable size for your memory limit.

@TheAndrey
Copy link
Author

TheAndrey commented Aug 3, 2023

How do I disable the use of this library? This only creates problems. I don't need to work with large numbers.
I didn't choose Laravel to invent bicycles to solve such simple tasks.

@timacdonald you have very skillfully shown the absence of error, showed examples on relatively small numbers. I don't see a solution to the application attack problem yet. Hackers will definitely use this error for a DoS attack.

I found commits in which a third-party library was added. Their rollback solves my problem.

Why were these changes made? Work with the extension can be made optional by checking it with extension_loaded() in the code.

@timacdonald
Copy link
Member

Hey @TheAndrey,

This library was introduced as PHP cannot handle arbitrarily large numbers.

The number you are working with, for example, is cast to 0.0 in PHP - although precision is system dependent - thus it is not possible to correctly compare max / min values with or to do comparisons against without a library like brick/math.

php -r "var_dump(floatval('10.9e-10000000000') === 0.0);"

// true

For now, I would recommend using a regex rule in conjunction with the bail rule.

If you do not want to support scientific notation input, once #47954 is merged I would recommend using something like:

"bail|required|decimal:0,5"

I also have a follow up PR that will extend the digits, max_digits, and min_digits to work in conjunction with the decimal rule.

This will allow you to limit the precision and overall size of the number in length.

"bail|required|decimal:0,5|max_digits:10"

I'll reopen this issue until we have both of these PRs merged.

@timacdonald timacdonald reopened this Aug 4, 2023
@TheAndrey
Copy link
Author

I know that PHP does not know how to work with large numbers. This programming language is not suitable for scientific computing.

thus it is not possible to correctly compare max / min values with or to do comparisons

Usually, rules are often set to the minimum value, this is a positive number.

What does it matter what the original number was if after the conversion, taking into the limitations of PHP, it can pass the validation rule? It looks incorrect in relation to the user when the number has changed, but at the same time the user does not have the opportunity to go beyond the acceptable values.

The brick/math library successfully copes with parsing large numbers, a memory error occurs during comparison operations. I think it is important to include into unit tests cases of working with huge numbers presented by me in this issue. This is an important mistake that cannot be ignored.

@timacdonald
Copy link
Member

This proposed validation rule will allow you to limit the length of a decimal: #47976

With this in place, you should be able to configure the rules to suit your system and avoid the memory issues you were hitting.

I have added unit tests to ensure that scientific notation is not accepted by the decimal rule.

Thanks for reporting this, @TheAndrey.

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

No branches or pull requests

2 participants