[M-01] Liquidation process will get `Dos'ed` if `tvl` falls below `DYAD::totalSupply`

[Al-Qa-qa]

This issue was introduced when mitigation issue H-08 from code4rena contest.

Description

As stated in issue H-08 in Code4rena contest, if the tvl in the Vaults is less than the DYAD totalSupply. KerosineVault::assetPrice() will revert because of underflow, which will lead to reverting when withdrawing Kerosine.

This issue introduced two impacts. First, is that the kerosine will get locked until the tvl becomes greater than DYAD totalSupply. and the liquidation process will get Dos'ed, as the function will revert when trying to fire KerosineVault::assetPrice() because of underflow.

As stated in issue 244 in code4rena contest, the liquidation process will get Dos'ed, and the mitigation was to let the function return 0 if tvl < totalSupply.

The issue is that this mitigates the issue only for the old implementation of liquidate(), but for the new implementation of the liquidate() it will not mitigate it. and the liquidation will still be Dos'ed.

In the new implementation of liquidate function we are dividing by vault.assetPrice(), so the liquidate will still not be possible but instead of underflow error, it will revert because of division by zero error.

VaultManagerV3.sol#L200-L203

          uint asset = cappedValue 
                         * (10**(vault.oracle().decimals() + vault.asset().decimals())) 
@>                       / vault.assetPrice() 
                         / 1e18;

As I stated, the main issue, which was in code4rena, introduces two impacts. temporal locked of kerosine, and liquidation Dos'ed. but in this case, only the liquidate function will get Dos'ed.

Although redeem() will get reverted because of division by zero, the withdraw() function will be callable. so I downgrade the severity from HIGH to MEDIUM.

Recommended Mitigation

if vault::assetPrice() returned 0 skip the current for-loop iteration.

VaultManagerV3.sol#L195

  function liquidate( ... ) ... {
      ...
      for (uint i = 0; i < numberOfVaults; i++) {
        Vault vault = Vault(vaults[id].at(i));
        if (vaultLicenser.isLicensed(address(vault))) {
          uint value       = vault.getUsdValue(id);
+        if (value == 0) continue;
          ...
        }
      }

      emit Liquidate(id, msg.sender, to);
  }

[shafu0x]

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

[Al-Qa-qa]

The issue has been fixed as recommended.