Fix for SOL-008 #3348
Fix for SOL-008 #3348
Conversation
libsolidity/ast/Types.cpp
Outdated
| bigint exp = bigint(string(expPoint + 1, _literal.value().end())); | ||
|
|
||
| if (exp > numeric_limits<int32_t>::max() || exp < numeric_limits<int32_t>::min()) | ||
| // SOL-008: Huge values for exponent are causing a crash. Limiting maximum to rational(1) << 4096 |
There was a problem hiding this comment.
We could shorten this to something like `disallow gigantic exponents to limit memory consumption'.
libsolidity/ast/Types.cpp
Outdated
| // SOL-008: Huge values for exponent are causing a crash. Limiting maximum to rational(1) << 4096 | ||
| // 1<<1024 = 1.7977^308. Extrapolating for 1<<4096, the exponent value will approximately be 1232. | ||
| // However, it does appear that values in excess of E77 will still cause casting errors. | ||
| if (exp > 1232 || exp < -1232) |
There was a problem hiding this comment.
In the end, this still depends on the mantissa. We could be conservative with the exponent, i.e. allow anything where abs(exp) < 1250 and then re-check once we have the actual value. We should re-check this anyway at the end of the function because you can also do things like
x = (1e20 * 1e20)** 500.
The only reason we need a more intricate check at any point where boost::multiprecision::pow is used is that this operation itself could already exhaust memory.
There was a problem hiding this comment.
Going by the following error message:
biginttest.sol:6:11: Error: Type int_const 1000000000000000000000000000000000000000000000000000000000000000000000000000000 is not implicitly convertible to expected type uint256. c=1E78;
It appears that we are running into the hard limit of uint256 (2^256 = 1.157920892×10⁷⁷.) Exponent 77. This being the case, why not just set the upper limit to 77? We are not going to be able to support anything bigger unless we start using a different data type.
There was a problem hiding this comment.
The idea is to allow intermediate values that are larger, as long as the final value of a computation fits into 256 bits.
|
Do you want to handle things like |
|
Oh and please add a test for this change. |
|
Oh I'm sorry, I forget something: Could you also add a line into the Changelog.md file at the topmost section under 'bugfixes'? Something like |
|
Could you please rebase this? There is a conflict in the changelog. |
|
Also can you please squash some of these commits? There appears to be a couple of them with the same commit log. |
|
Done. Sorry had some difficulties getting used to remote branch :) |
|
Please remove the merge commits by rebasing. |
Cleaner fix for exponent evaluation
|
This has gotten completely out of hand. my fault. I'll submit a fresh request with the proper fix. |
|
I thought I closed this out... ah well. closing this out for a cleaner PR. |
4 participants
Fix for issue with very large exponent values causing CPU blowout. Exponent values now restricted to max of 1232 ( rational(1) << 4096, approximated.) It should also be noted that values above E78 are causing casting errors:
biginttest.sol:6:11: Error: Type int_const 1000000000000000000000000000000000000000000000000000000000000000000000000000000 is not implicitly convertible to expected type uint256. c=1E78;