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

2 stars 1 forks source link

Lack of slippage check in `mint()` can result in loss of funds #524

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#L776

Vulnerability details

Users can call VToken.mint to deposit underlying and mint vTokens.

The amount of vToken minted is proportional to the actualMintAmount of underlying they transferred:

776: uint256 mintTokens = div_(actualMintAmount, exchangeRate); 

The issue is that this operation can result in mintTokens == 0 if actualMintAmount < exchangeRate

As there is no further check that mintTokens != 0, the call will succeed, without updating accountTokens[minter]

Impact

A user in this situation will have lost their underlying - as their accountTokens[user] mapping would still be 0.

Note that based on the high value of the initial ExchangeRateMantissa

PoolRegistry.sol
290: 10**(underlyingDecimals + 18 - input.decimals),

This situation is possible

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add a check at the end of _mintFresh to ensure mintTokens != 0

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-c