code-423n4 / 2023-05-venus-findings

2 stars 1 forks source link

`Comptroller.exitMarket()` does not get an updated `exchange rate` causing inacurate exit validations #502

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Comptroller.sol#L187 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Comptroller.sol#L1240 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Comptroller.sol#L1296 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Comptroller.sol#L1402 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L1463 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/VToken.sol#L678

Vulnerability details

Impact

The user can use the exitMarket() function to remove an asset from the account liquidity calculation; disabling them as collateral.

The problem is that the user can call exitMarket() before the vToken updates their accrue interests via accrueInterest() function causing that the user can exit from the market even if he has debts snapshot.shortfall > 0.

The interests are increased when the chain block increases. If the exitMarket() is called before anyone can call accrueInterest(), the exitMarket() function will have outdated data to validates if the user can exit the market or not.

Proof of Concept

Please see the next path execution:

  1. The exitMarket() calls _checkRedeemAllowed()
  2. The _checkRedeemAllowed() function calls _getHypotheticalLiquiditySnapshot() function.
  3. The _getHypotheticalLiquiditySnapshot() function calls _safeGetAccountSnapshot().
  4. The _safeGetAccountSnapshot() function calls vToken.getAccountSnapshot()
  5. The vToken.getAccountSnapshot() calls _exchangeRateStored() function.
  6. The _exchangeRateStored() function calculates the exchange rate based on the totalBorrows. The exchange rate here could be outdated because the vToken.accrueInterest() is responsable to calculate the new total borrows and it is possible to call _exchangeRateStored() before anyone can call accrueInterests().
  7. The outdated exchange rate calculated will be used in the vToken price calculation causing to have innacurate information to decide if the borrower is shortfall or not.
  8. The _checkRedeemAllowed() function may evaluate the shortfall check incorrectly.

The interests are increased by the block delta. So the exitMarket() must calculate the current block interests to be able to check if the user is able to exit the market or not.

Tools used

VSCode

Recommended Mitigation Steps

Accrue the vToken interests accrueInterest() once the user call exitMarket() function so the function can get updated info and it is possible to validate if the user is able to exit or not with updated info.

Assessed type

Other

c4-judge commented 1 year ago

0xean marked the issue as duplicate of #104

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

c4-judge commented 1 year ago

0xean marked the issue as duplicate of #486