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

2 stars 1 forks source link

Using old oracle prices for estimation users assets before redeeming #565

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-L199 https://github.com/code-423n4/2023-05-venus/blob/8be784ed9752b80e6f1b8b781e2e6251748d0d7e/contracts/Comptroller.sol#L292-L299

Vulnerability details

Impact

Users can avoid correct estimation of assets and redeem more tokens than would redeem in case of estimation with updated oracle prices.

Proof of Concept

exitMarket function doesn't call oracle.updatePrice before _checkRedeemAllowed check at all. preRedeemHook and preTransferHook call oracle.updatePrice before _checkRedeemAllowed check, but only for redeemed VToken. The _checkRedeemAllowed check estimates the user assets liquidity through the _getHypotheticalLiquiditySnapshot function. _getHypotheticalLiquiditySnapshot calculates snapshot.shortfall with oracle.getUnderlyingPrice(address(asset)) from _safeGetUnderlyingPrice function. The oracle.updatePrice should be called every time before calling oracle.getUnderlyingPrice for every token but it doesn't in case of Comptroller.sol.

Tools Used

Manual review

Recommended Mitigation Steps

I suggest calling oracle.updatePrice for all assets from users accountAssets in _getHypotheticalLiquiditySnapshot function.

Assessed type

Oracle

c4-judge commented 1 year ago

0xean marked the issue as duplicate of #88

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

c4-judge commented 1 year ago

0xean changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

0xean marked the issue as duplicate of #486

c4-judge commented 1 year ago

0xean marked the issue as partial-50