code-423n4 / 2022-07-yield-findings

0 stars 0 forks source link

Incorrect payout calculation due to a division before multiplication #141

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-yield/blob/6ab092b8c10e4dabb470918ae15c6451c861655f/contracts/Witch.sol#L591

Vulnerability details

Impact

Reverse dutch auction price is calculated incorrectly

Proof of Concept

In the function _calcPayout which calculates reverse dutch auction according to the formula

inkOut = (artIn / totalArt) * totalInk * (p + (1 - p) * t)

t is always zero because you divide before multiply.
In the else clause elapsed is lower than duration so elapsed/duration is zero .

} else if (elapsed > duration) {
  proportionNow = 1e18;
} else {
  proportionNow =
    uint256(initialProportion) +
    uint256(1e18 - initialProportion).wmul(elapsed.wdiv(duration));@audit high "elapsed < duration so division is zero"
}

This means that the price will be initialPropotion until the end where it will be 100%

Recommended Mitigation Steps

Use the formula

 proportionNow =
    uint256(initialProportion) +
    uint256(1e18 - initialProportion).wmul(elapsed).wdiv(duration)
alcueca commented 2 years ago

That's wdiv and wmul, which are fixed point operations and don't have this problem.

elapsed.wdiv(duration) is equivalent to elapsed * 1e18 / duration

PierrickGT commented 2 years ago

Agree with sponsor, elapsed being scaled to 1e18, elapsed should always be greater than duration and so the division should never return 0. For this reason, I have labelled this issue as invalid.