Auctioneer Cut calculated in different order of magnitude

[code423n4]

Lines of code

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

Vulnerability details

Impact

Auctioneer fee is calculated in different order of magnitude.

Proof of Concept

As your are defining auctioneerReward in 1e18 basis point (being 1e18 100%) you need to divide by 1e18 when calculating percentage.

function setAuctioneerReward(uint128 auctioneerReward_) external auth {
    if (auctioneerReward_ > 1e18) {
        revert AuctioneerRewardTooHigh(1e18, auctioneerReward_);@audit medium auctioneer reward set to 100%
    }
    auctioneerReward = auctioneerReward_;
    emit AuctioneerRewardSet(auctioneerReward_);
}

However in this line you are missing that division which makes the transaction revert for underflow in best scenario.

if (auction_.auctioneer != to) {
        auctioneerCut = liquidatorCut.wmul(auctioneerReward);@audit  auctioneer reward missing division
        liquidatorCut -= auctioneerCut;
    }

Recommended Mitigation Steps

Replace

[-] auctioneerCut = liquidatorCut.wmul(auctioneerReward);
[+] auctioneerCut = liquidatorCut.wmul(auctioneerReward).wdiv(1e18);

[KenzoAgada]

The wmul function divides by 1e18. See yield-utils.

library WMul {
    // Taken from https://github.com/usmfum/USM/blob/master/contracts/WadMath.sol
    /// @dev Multiply an amount by a fixed point factor with 18 decimals, rounds down.
    function wmul(uint256 x, uint256 y) internal pure returns (uint256 z) {
        z = x * y;
        unchecked { z /= 1e18; }
    }
}

[alcueca]

Disputed, @KenzoAgada explained why.

[PierrickGT]

As mentioned by Kenzo, wmul does divide/scale down by 1e18, so the division mentioned by the warden is already in place. For this reason, I have labelled this issue as invalid.