Yield Witch v2 contest - hyh's results

Lines of code

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

Vulnerability details

Precision is now can be lost due to division done in _calcPayout() ahead of multiplication in resulting collateral amount calculation.

_calcPayout() is used in payBase() and payFYToken(), also in calcPayout() for calculation of the liquidator and auctioneer cuts.

Setting the severity to medium as that can constitute diminished returns for liquidator and auctioneer compared to the stated linear proportion, i.e. the system behavior diverts from the expected and produces losses for counterparts.

Proof of Concept

Resulting collateral amount in _calcPayout() is calculated by performing the division first:

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

uint256 inkAtEnd = uint256(artIn).wdiv(auction_.art).wmul(auction_.ink);

This constitutes (artIn * 1e18) / auction_.art * auction_.ink / 1e18 (1) expression.

https://github.com/yieldprotocol/vault-v2/blob/d8e26b3ac4bc6bbef64fa7f2157a58c88ab3b8ee/packages/foundry/lib/yield-utils-v2/contracts/math/WMul.sol#L8-L10

https://github.com/yieldprotocol/vault-v2/blob/d8e26b3ac4bc6bbef64fa7f2157a58c88ab3b8ee/packages/foundry/lib/yield-utils-v2/contracts/math/WDiv.sol#L8-L9

Rewriting this with multiplication by auction_.ink first yields artIn * auction_.ink / 1e18 * 1e18 / auction_.art (2).

As fyToken amount auction_.art is generally greater than 1e18 (i.e. usual sum is greater than 1 unit; can be magnitudes greater), the precision is reduced by doing the division first.

As a corner case example, if artIn = 1e18, auction_.art = auction_.ink = 1e19*e18, (1) is (1e18 * 1e18) / (1e19*1e18) * (1e19*1e18) / 1e18 = 0, (2) is 1e18 * (1e19*1e18) / 1e18 * 1e18 / (1e19*1e18) = 1e18, i.e. inkAtEnd will be rendered to 0.

inkAtEnd is then used for liquidator and auctioneer rewards calculation:

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

        uint256 inkAtEnd = uint256(artIn).wdiv(auction_.art).wmul(auction_.ink);
        liquidatorCut = inkAtEnd.wmul(proportionNow);
        if (auction_.auctioneer != to) {
            auctioneerCut = liquidatorCut.wmul(auctioneerReward);
            liquidatorCut -= auctioneerCut;
        }

Recommended Mitigation Steps

Consider switching to an equivalent expression that have multiplication performed first:

-   uint256 inkAtEnd = uint256(artIn).wdiv(auction_.art).wmul(auction_.ink);
+   uint256 inkAtEnd = uint256(artIn).wmul(auction_.ink).wdiv(auction_.art);