Vulnerability: Donation Attacks can Cause Loss of Liquidity and/or Undesired Prices Rebalance / Contract: GeVault / Function: withdraw

[code423n4]

Lines of code

https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L214-L241

Vulnerability details

Impact

Donation attack can cause loss of users liquidity or undesired tick rebalance by price manipulation.

Proof of Concept

An attacker can cause constant rebalancing, instability, and along with the vulnerability of slot0 in the getTokenAmountsExcludingFees function, manipulation of ticks and prices by sending a different token since it is not validated that the token sent to the withdraw function is one of the valid GeVault tokens.

Scenario 1:

1. An attacker has liquidity in the vault and calls to withdraw with the address of another token (other than token0/token1) of which the attacker previously transfer an amount to the vault or another user may have mistakenly sent a transfer of that other token to the vault. (L214) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L214
2. As the storage variables token0/token1 are used in a uniswap pool and a chainlink oracle, the prices are valid and it passes the poolMatchesOracle require. (L215) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L215C8-L215C8
3. Liquidity is validated because the attacker has liquidity in the vault, but the token is not validated to be a valid token in the vault. (L216-218) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L216-L218
4. The amount and fee are calculated correctly, since the other token is a valid token in a chainlink oracle. (L220-223) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L220-L223
5. User liquidity is burned. (L225) and a rebalancing is started by removing all ticks. (L226) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L225-L226
6. the fee would be paid to the treasury in the token sent by the attacker or user. (L227) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L227
7. The payment (bal = amount - fee) would be transferred to the attacker in the sent token. (L235) https://github.com/GoodEntry-io/ge/blob/8a2686b14114edbd1ec523d79304ed678cc2e915/contracts/GeVault.sol#L234-L236
8. If the attacker manipulates the uniswap pool with a flashloan/flashswap in conjunction with the slot0 vulnerability in the function getTokenAmountsExcludingFees it can cause unwanted rebalancing of undesired prices.

Scenario 2:

1. A user mistakenly transfers a token other than token0 and token1 or an attacker transfers an amount of several different tokens.
2. If a normal user wants to withdraw his liquidity and mistakenly sends the address of another token (one of the tokens sent in the previous step).
3. this user would be losing/burning their liquidity and getting an unwanted token that could be worth less than their liquidity.

function withdraw(uint liquidity, address token) public nonReentrant returns (uint amount) {
    require(poolMatchesOracle(), "GEV: Oracle Error");
    if (liquidity == 0) liquidity = balanceOf(msg.sender);
    require(liquidity <= balanceOf(msg.sender), "GEV: Insufficient Balance");
    require(liquidity > 0, "GEV: Withdraw Zero");

    uint vaultValueX8 = getTVL();
    uint valueX8 = vaultValueX8 * liquidity / totalSupply();
    amount = valueX8 * 10**ERC20(token).decimals() / oracle.getAssetPrice(token);
    uint fee = amount * getAdjustedBaseFee(token == address(token1)) / 1e4;

    _burn(msg.sender, liquidity);
    removeFromAllTicks();
    ERC20(token).safeTransfer(treasury, fee);
    uint bal = amount - fee;

    if (token == address(WETH)){
      WETH.withdraw(bal);
      payable(msg.sender).transfer(bal);
    }
    else {
      ERC20(token).safeTransfer(msg.sender, bal);
    }

    // if pool enabled, deploy assets in ticks, otherwise just let assets sit here until totally withdrawn
    if (isEnabled) deployAssets();
    emit Withdraw(msg.sender, token, amount, liquidity);
  }

Tools Used

Manual Analysis

Recommended Mitigation Steps

Validate at the beginning of the function that the address sent by the user is one of the valid tokens for the vault (token0 or token1)

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as duplicate of #48

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Invalid