Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

p12473 - DOS via share inflation attack #125

Closed
sherlock-admin opened this issue Jun 29, 2023 · 0 comments
Closed

p12473 - DOS via share inflation attack #125

sherlock-admin opened this issue Jun 29, 2023 · 0 comments
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Jun 29, 2023

p12473

medium

DOS via share inflation attack

Summary

A malicious user can DOS the vault by performing a share inflation attack. While a share inflation attack might seem to be mitigated by hardcoding an initial denominator of 1 ether and specifying the vault’s init0/1s, it is still possible to bypass this.

Vulnerability Detail

This explanation assumes the following:

  • For ease of explanation, vaultV2.init0 and vaultV2.init1 have the same (appropriately set) values.
  • Malicious user can leverage on flashbots to backrun the deployment transaction to ensure that his attack will be executed immediately after the vault has been minted and configured.

This vulnerability can be broken down into the 3 steps:

1. Malicious user makes the first mint

The malicious user will call ArrakisV2Router.addLiquidity with the following arguments:

addLiquidity(
	AddLiquidityData({
		amount0Max: 1, // or a greater value if `vaultV2.init0()` is greater than 1 ether
		amount1Max: 1, // or a greater value if `vaultV2.init1()` is greater than 1 ether
		amount0Min: 1,
		amount1Min: 1,
		amountSharesMin: 1,
		vault: address(vaultV2),
		receiver: address(malicious_user),
		gauge: address(0),
	})
)

This will pass the various validation checks and will mint the malicious user with 1 wei worth of shares.

Note that inside of UnderlyingHelper.computeMintAmounts(), amount0/1Max should be configured such that the numerator (amount0Max * totalSupply_) is greater than current0/1_ or the require(sharesReceived > 0, "nothing to mint"); will revert. For ease of explanation, let’s assume that init0/1 is less than 1 ether.

2. Malicious user burns everything but 1 wei of share

For example, if init0/1 is 0.01 ether, the calculated mintAmount for the first mint is 100 so the malicious user would burn 99 wei of shares. The malicious user will call ArrakisV2Router.removeLiquidity with the following arguments:

removeLiquidity(
	RemoveLiquidityData({
		burnAmount: 99,
		amount0Min: 0,
		amount1Min: 0,
		vault: address(vaultV2),
		receiver: address(malicious_user),
		gauge: address(0),
		receiveETH: false,
	})
)

Since the malicious user only deposit 1 wei of token to mint 100 shares, burning 99 shares will result in a value less than 1 wei of token so it rounds down to 0. There are no checks to revert when amount0/1 is 0 so the malicious user is able to remove all but 1 wei of shares.

3. Malicious user deposits directly into the vault contract

The malicious user would deposit directly into the Vault contract. The reason why this works is when amount0/1 is calculated inside of Underlying.totalUnderlyingForMint(), it takes into account the balance of the contract. This makes sense because the fees collected from every other users’ positions are residing in the vault contract however in this case, there is only 1 user and 0 fee collected so far. The malicious user is able to inflate the balance of the contract so that in the future when someone else wants to mint, they need to mint shares at the inflated rate.

Impact

If the vault only has 1 user (the malicious user), it should be quite trivial to redeploy however if there are a few more users who are willing to pay an exorbitant rate to get 1 wei worth of vault tokens then it makes it more troublesome to “fix” this. In both cases, users who cannot pay this rate will not be able to use the vault.

Since no funds are lost as a result of this vulnerability, I have marked this as a medium vulnerability.

Code Snippet

https://github.com/ArrakisFinance/v2-core/blob/9133fc412b65c7a902f62f1ad135f062e927b092/contracts/libraries/Underlying.sol#L72-L90

Tool used

Manual Review

Recommendation

At initialization, mint some shares to a dead address. You can either offset this loss to the first minter (e.g. like how uniswap v2 does it) or have the protocol pays for this. There are also other mitigation strategies discussed here. Also add an additional require check to prevent amount0/1 from being 0 when burn is called. This will help to prevent users from burning their shares but not getting any tokens back.

Duplicate of #266

@github-actions github-actions bot closed this as completed Jul 3, 2023
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jul 3, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant