Submitted by iamandreiski, also found by 0xAlix2
https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L856-L866
https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1197-L1202
https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1270-L1278
https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L702-L703
https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1090-L1120
The core mechanism that Revert utilizes to prevent arbitrary tokens to be utilized as collateral is by the default setting of the collateralFactor. Since the collateralFactor for all tokens (besides the ones approved for usage in the system, and set by admins) is 0, that means that no one can borrow against a collateral which hasnt been approved.
The problem arises when admins would want a collateral removed from the system. There would be multiple reasons as to why this might be the case:
The only way in which this could be performed is to set the collateralFactor back to 0, but this would break core mechanics such as liquidations.
All approved collateral which admins decided should be utilized inside of the protocol is introduced by increasing the collateralFactor through setTokenConfig():
Once a tokens collateralFactor was set, and removed afterward due to extraordinary circumstances, all outstanding loans will never be able to be liquidated either due to panic reverts because of overflow/underflow or panic reverts due to division/modulo by 0.
The problem arises when _checkLoanIsHealthy() is called within liquidate() (the same can be tested by calling loanInfo() as well, since it also calls _checkLoanIsHealthy()).
This happens because when calculating the collateralValue through _checkLoanIsHealthy as it can be seen here:
Since its needed to calculate the liquidation collateral value as it can be seen in the liquidate() function:
Since the collateral factor will be 0, the collateralValue will also be 0, this will lead to passing 0 as a value in the _calculateLiquidation() function:
Which will always revert due to division with 0:
This can be tested via PoC, by inserting the following line in the testLiquidation() test in V3Vault.t.sol:
vault.setTokenConfig(address(USDC), 0, type(uint32).max); vault.setTokenConfig(address(DAI), 0, type(uint32).max)
You can change USDC/DAI one or both with whatever collateral is part of the NFT in question and run testLiquidationTimeBased().
Dont use the collateralFactor as the common denominator whether a token is accepted as collateral or not; use a method such as whitelisting tokens by address and performing necessary checks to see if the token address matches the whitelist.
DoS
kalinbas (Revert) confirmed, but disagreed with severity
ronnyx2017 (judge) decreased severity to Medium and commented:
Revert mitigated:
Status: Mitigation Confirmed. Full details in reports from b0g0, thank_you and ktg.
