Open cyberthirst opened 5 months ago
I think that
@external
def div_bug() -> int256:
return -2**255 / -1
not compiling is a feature and not a bug, in the same way, safe_div
is reverting in those case:
https://github.com/vyperlang/vyper/blob/e1adb7b3344c1ac03facfa553830a94dd7def2e2/vyper/codegen/arithmetic.py#L315
All other examples compiling are caused by issues with the old constant folding not doing proper type checking (https://github.com/vyperlang/vyper/issues/2830), none of them should compile in master
from what I've tested.
An idea to have a better exception however would be to have something like that in FloorDiv._op()
:
if left == -2**255 and right == -1:
raise OverflowException("Cannot divide min_value(int256) by -1", self)
2**255
and -2**255
are the same on the evm.
Submitted by Franfran._
Relevant GitHub Links
https://github.com/vyperlang/vyper/blob/3b310d5292c4d1448e673d7b3adb223f9353260e/vyper/ir/optimizer.py#L54-L55
https://github.com/vyperlang/vyper/assets/51274081/3f619c79-88e0-4d15-9ace-7d9ba02d16bc
Summary
At compile-time, division is using the same logic for both signed and unsigned integers. This is causing some corectness issues.
Vulnerability Details
Compile time div operation for both signed and unsigned are defined by
evm_div
But there should be an edge case according to the Ethereum yellow paper:
As you can see,
DIV
andSDIV
are not purely equivalent. There is an edge case when $\mu[0] = -2^{255}$ and $\mu[1] = -1$. If we evaluate the expression with the Python engine, this is what we get for this function:It's
2**255
, while it should be-2**255
.Impact
Here are some examples at looking how this could be exploited:
Doesn't work, caught by the type checker:
While it should compile.
But we can for instance make it compile this way, while it should revert since
as_wei_value
does not support negative values.This compiles while the value should evaluate to a negative value, and returns
0x8000000000000000000000000000000000000000000000000000000000000000
.Another example:
returns
0x8000000000000000000000000000000000000000000000000000000000000000
becausemax
is evaluated at compile-time with the wrong computation of-2**255 / -1
. The expected result should be0
.returns
0
Other things that compile while it shouldn't:
Tools Used
Manual review
Recommendations
(might be better to create a
evm_sdiv
to make sure that it won't cause any issue in the future)