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

2 stars 1 forks source link

Users can lose their `vToken` in `redeem` #531

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/VToken.sol#L825

Vulnerability details

Users can redeem their vToken against underlying by calling redeem. The logic in _redeemFresh to compute that amount is:

824: redeemTokens = redeemTokensIn;
825:             redeemAmount = mul_ScalarTruncate(exchangeRate, redeemTokensIn);

The issue is that this will be equal to 0 if exchangeRateStored()*redeemTokensIn < 1e18.

Impact

In this case, the problem is that the code will not revert. The only check is that redeemAmount is not zero if redeemTokens is 0, but not the other way around:

VToken.sol
837: if (redeemTokens == 0 && redeemAmount > 0) {
838:             revert("redeemTokens zero");
839:         }

This will only happen for dust amounts of VTokens.

The issue is that exchangeRate is dynamic, and cannot be fully predicted, which means a redeem() call may lead to a lower underlying amount than expected.

Tools Used

Manual Analysis

Recommended Mitigation Steps

VToken.sol
+837: if ((redeemTokens == 0 && redeemAmount > 0) && (redeemTokens != 0 && redeemAmount == 0){
-837: if (redeemTokens == 0 && redeemAmount > 0){
838:             revert("redeemTokens zero");
839:         }

Assessed type

Math

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-b