code-423n4 / 2021-07-sherlock-findings

0 stars 0 forks source link

order of operations in Payout.sol #130

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xsanson

Vulnerability details

Impact

The expression excludeUsd.div(curTotalUsdPool.div(SherXERC20Storage.sx20().totalSupply)) computes a division of a division: it's possible to write it as a multiplication and a division. We gain a lot of precision this way, since there is a risk that the second division gives 0, reverting the total computation.

Proof of Concept

https://github.com/code-423n4/2021-07-sherlock/blob/main/contracts/facets/Payout.sol#L185

Tools Used

editor

Recommended Mitigation Steps

Write the expression as excludeUsd.mul(SherXERC20Storage.sx20().totalSupply).div(curTotalUsdPool).

Evert0x commented 3 years ago

It's actually impossible for SherXERC20Storage.sx20().totalSupply to be zero at that point.

ghoul-sol commented 3 years ago

per sponsor comment, invalid