Yield Witch v2 contest - kenzo's results

Lines of code

https://github.com/code-423n4/2022-07-yield/blob/main/contracts/Witch.sol#L253

Vulnerability details

When an auction is going on, after it started, the vault might become overcollateralized again, due to liquidators buying early (and not receiving the full amount of collateral), or due to user supplying collateral to the vault.

In these cases (especially the first one), the vault might become overcollateralized, but the auction would be still be going on and bids allowed to be placed.

The only way to cancel an auction is either using the external cancel function or by paying all of the debt.

Impact

More collateral would be liquidated than needed to keep the vault's collateralization ratio.

I am not sure if this is intended behavior. But seems like this will hurt the users. The contest readme says about WitchV1 "we have found that liquidations have not been as fair as we would have liked. More often than not, liquidated users have lost all their collateral". Therefore it seems you might want to not liquidate more than necessary.

Proof of Concept

For liquidators, the auction mechanism gives out less collateral than debt payed at the beginning of the auction:

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

(where t is % of time elapsed)

Therefore, according to the parameters used, a liquidator buying collateral may end up putting the vault over the collateralization ratio. At that point, the vault is not in a state to be liquidated anymore, but it's auction is still ongoing. Liquidators have no incentive to cancel it, and may just continue bidding, buying more of the user's collateral, making him lose even all of it (according to the auction parameters).

(Additionally, there's a side case where a user might repay his debt, and liquidators can front run his cancel tx to continue liquidating him - but that might be less of an issue as Yield expects users to know what they're doing.)

Recommended Mitigation Steps

Revert in payFYToken and payBase if cauldron.level(vaultId) >= 0. You can even cancel the auction at that stage but that's going the extra mile.