search

code-423n4 / 2021-10-tracer-findings

0 stars 0 forks source link

Unsafe `int256` casts in `executePriceChange` #14

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

LeveragedPool.executePriceChange function performs unsafe casts. If the unsigned values are above the maximum signed value (type(int256).max), it will be interpreted as a negative value instead.

emit PoolRebalance(
    int256(newShortBalance) - int256(_shortBalance),
    int256(newLongBalance) - int256(_longBalance)
);

Impact

The values in the event might be wrong and an off-chain app might be tricked.

Recommended Mitigation Steps

Make sure the value fits into the type first by using a SafeCast library.

kumar-ish commented 3 years ago

Valid issue as such, but the only way for this to happen is if the ERC-20 implementation of a token is half of the total possible supply in the maximum case (i.e. >= 2^255) AND for that to be deposited on either side of the pool.

Considering that the DAO/governance is the only one that can create markets, it would be incredibly unlikely/impossible for it to use a quote token that has that much supply (considering it's only a few orders of magnitudes less than the number of atoms in the universe); I'd suggest this be bumped down to an informational. We're not going to fix this due to the extremely marginal benefit and the gas cost of that checking being more than that marginal benefit.

GalloDaSballo commented 3 years ago

Agree with sponsor on the marginality of the finding, recommend you do keep this in mind for any potential gotchas, especially with memecoins

Agree on reducing to informational