Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EXPLOITABLE VULNERABILITY IN FIRST ASSET SUPPLY IMPACTING VTOKEN CALCULATIONS #462

Closed
code423n4 opened this issue May 15, 2023 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-220 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L743-L791
https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L800-L871
https://github.com/code-423n4/2023-05-venus/blob/main/contracts/VToken.sol#L1463-L1482

Vulnerability details

Impact

A serious vulnerability has been identified in the VToken contract within the Venus protocol. This vulnerability, stemming from an exploit in the initial supply of asset tokens, allows for manipulation of the exchangeRate, potentially leading to profitable exploits at the expense of subsequent users. This is typically seen in share-based liquidity pools or vault contracts where the first investor can manipulate the price per share, thereby profiting from the next deposit due to precision loss induced by the inflated value of the price per share.

The vulnerability lies in the fact that the first investor has the potential to maliciously manipulate the exchangeRate by supplying the smallest feasible amount of the asset, thereby artificially inflating the exchangeRate. Specifically, because of the default rounding down associated with division truncation, this initial deposit could be tailored to front-run someone trying to make the first mint, thereby maximizing the profitability of the exploit.

Proof of Concept

Here's a typical exploit scenario, assuming initialExchangeRateMantissa = 1e18 (as assigned in TokenTestHelpers.ts) and totalBorrows, badDebt and totalReserves remain in their defaulted zeros values:

  1. AlEX, a user, wishes to supply 1000e18 DAI to an empty VToken contract using DAI (compliant with 18 decimals) as its asset token.

  2. Observing this transaction in the mempool, Lori, an attacker, decides to front-run it by depositing an initial liquidity of 1 wei DAI via mint(). This results in an exchange rate of 1:1 for wei vToken to wei DAI.

  3. Lori then artificially inflates the asset token balance by transferring 1000e18 DAI directly to the VToken contract without minting any new vToken.

  4. The exchange rate is now 1 wei vToken per (1000e18 + 1) DAI.

  5. Alex's deposit of 1000e18 DAI goes through, but due to precision loss, she receives 0 vToken and effectively loses her entire fund.

  6. Lori could repeat this process with other victims, waiting for more deposits of assets less than _getCashPrior(), to receive 0 vToken like Alex, until she decides to call redeem() and receive all the underlying DAI.

Recommended Mitigation Steps

To mitigate this vulnerability, consider implementing the Uniswap V2 protocol's approach of sending the first 1000 wei of vToken to address 0 when _totalSupply == 0.

Apart from that, a minimum deposit amount could also be established to prevent an attacker from manipulating the exchange rate by depositing a minimal amount of DAI. The limit should be set sufficiently high to prevent inflation of the exchangeRate to an extent that would cause precision issues.

Furthermore, the protocol could explore the implementation of slippage protection measures to further safeguard against such scenarios.

Assessed type

ERC4626

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels May 15, 2023
code423n4 added a commit that referenced this issue May 15, 2023
@c4-judge
Copy link
Contributor

0xean marked the issue as duplicate of #314

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jun 5, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Jun 5, 2023

0xean marked the issue as satisfactory

@c4-judge
Copy link
Contributor

c4-judge commented Jun 5, 2023

0xean changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge labels Jun 5, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Jun 5, 2023

0xean changed the severity to 2 (Med Risk)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-220 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants