Al-Qa-qa / dyad-private-audit

4 stars 1 forks source link

[L-03] The new liquidation implementation does not round in the favour of the liquidator #6

Open Al-Qa-qa opened 3 weeks ago

Al-Qa-qa commented 3 weeks ago

This issue introduced when mitigation issue M-01 from code4rena contest.

Description

liquidate() old implementation roundsUp (in the favour of liquidator), this is to secure the largest collateral transfer to the liquidator.

          uint  collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);

But in the new implementation, the rounding is Down when calculating shares and when calculating Assets to transfer.

VaultManagerV3.sol#L197-L198

@>        uint amountShare = share.mulWadDown(amount);
          uint valueToMove = amountShare + amountShare.mulWadDown(reward_rate);
          uint cappedValue = valueToMove > value ? value : valueToMove;
          uint asset = cappedValue 
                         * (10**(vault.oracle().decimals() + vault.asset().decimals())) 
                         / vault.assetPrice() 
                         / 1e18;

Recommended Mitigation

Rounding Up instead of rounding Down when calculating amountShare.

VaultManagerV3.sol#L197

-         uint amountShare = share.mulWadDown(amount);
-         uint valueToMove = amountShare + amountShare.mulWadDown(reward_rate);
+         uint amountShare = share.mulWadUp(amount);
+         uint valueToMove = amountShare + amountShare.mulWadUp(reward_rate);
shafu0x commented 2 weeks ago

fixed here: https://github.com/DyadStablecoin/contracts/pull/55

Al-Qa-qa commented 2 weeks ago

Issue has Been Fixed as Recommended.