code-423n4 / 2024-05-loop-findings

4 stars 4 forks source link

Users can exploit `claim()` to withdraw their locked tokens when they are not allowed. #3

Closed c4-bot-1 closed 5 months ago

c4-bot-1 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L240-L266

Vulnerability details

Impact

An malicious user with locked non-ETH tokens can exploit the claim() function to withdraw his locked tokens during situations where he should not be allowed to withdraw (block.timestamp >= startClaimDate and emergencyMode = false). By exploiting this, the user can deposit huge amount of tokens to get their off-chain points computed, and then retrieve his tokens back without needing to convert the locked funds to LpETH.

Proof of Concept

When block.timestamp >= startClaimDate and emergencyMode = false, users are unable to withdraw their locked tokens. However, any user can exploit the claim() function by using crafted _data arguments to effectively withdraw their locked tokens from the contract.

The claim() function calls _claim(), which, when dealing with non-ETH tokens, verifies the _data arguments (the data passed for the 0x Exchange Router). However, it only validates the input and output token addresses and input amount. It doesn't check if the data corresponds to a single hop swap or a multi hop swap. This allows any user to use multi hop swaps to retrieve their locked tokens without converting them to LpETH.

For example, let's say Alice wants to withdraw her swETH locked in the PrelaunchPoints contract. To exploit this, Alice needs to deploy an ERC20 token (tokenA), which she fully controls the supply, create two new UniswapV3 pools that she can manipulate the token balances, and execute a multi hop swap passing through those pools using the claim() function.

Each pool serves a purpose:

Alice can then craft her _data to pass all required validations done by _validateData() (see code below) and then call claim() using her crafted _data.

bytes4 selector = 0x803ba26d; //sellTokenForEthToUniswapV3() selector
address inputToken = 0xf951E335afb289353dc249e82926178EaC7DEd78; //swETH
address outputToken = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; // WETH
uint24 fee = 500;
bytes memory encodedPath = abi.encodePacked(inputToken, fee, address(tokenA), address(tokenA), fee, outputToken);
uint256 sellAmount = lockedswETHAmount;
uint256 minBuyAmount = 0;
address recipient = address(prelaunchPoints);
bytes memory _data = abi.encodeWithSelector(selector, encodedPath, sellAmount, minBuyAmount, recipient);

After the multi-hop swap (swETH - tokenA, then tokenA - WETH) executed during claim(), arbitrary amounts of swETH would be converted in dust amounts of WETH, which is then converted to LpETH, while Alice's swETH is now in the first pool balance. Note that the minBuyAmount field used by sellTokenForEthToUniswapV3() is also in _data and thus controlled by Alice, so this unfavorable swap succeeds if she sets minBuyAmount = 0.

Subsequently, after the claim() call, Alice can recover her swETH tokens by removing the liquidity from the first pool. As she is the sole liquidity provider to that pool, she is able to recover it all. Therefore, as described, any user is able to exploit the claim() function to withdraw their locked tokens.

Tools Used

Manual Review.

Recommended Mitigation Steps

Restrict the swaps to be single hop, which is not ideal as this may not be the most efficient swap path on some circumstances, or verify that the _data argument comes from Loop's off-chain services, for example by signing the data and validating the signing in the contract before executing the swap.

Assessed type

Other

c4-judge commented 5 months ago

koolexcrypto marked the issue as primary issue

0xd4n1el commented 5 months ago

This is technically possible, but we are tracking the Claim event for points calculation, so users cannot exploit this to get more points

c4-judge commented 5 months ago

koolexcrypto changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

koolexcrypto marked the issue as grade-c