howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L448-L464

Vulnerability details

Impact

UniswapV3 decodes the parameters for each swap pool in a multi-pool path bytes array in the following order: tokenA,fee,tokenB. Each token address is encoded by 20 bytes and fee by 3bytes. If there are remaining bytes in the path, they will be ignored. For example, if you provide path like tokenA,fee,tokenB,tokenC, this path will be treated as tokenA,fee,tokenB in UniswapV3's SwapRouter.

In PrelaunchPoints, the _decodeUniswapV3Data only parses the first and the last token from the path:

encodedPathLength := calldataload(add(p, 96)) // Get length of encodedPath (obtained through abi.encodePacked)
inputToken := shr(96, calldataload(add(p, 128))) // Shift to the Right with 24 zeroes (12 bytes = 96 bits) to get address
outputToken := shr(96, calldataload(add(p, add(encodedPathLength, 108)))) // Get last address of the hop

and then these tokens will be checked in _validateData:

if (_exchange == Exchange.UniswapV3) {
            (inputToken, outputToken, inputTokenAmount, recipient, selector) = _decodeUniswapV3Data(_data);
            if (selector != UNI_SELECTOR) {
                revert WrongSelector(selector);
            }
            // UniswapV3Feature.sellTokenForEthToUniswapV3(encodedPath, sellAmount, minBuyAmount, recipient) requires `encodedPath` to be a Uniswap-encoded path, where the last token is WETH, and sends the NATIVE token to `recipient`
            if (outputToken != address(WETH)) {
                revert WrongDataTokens(inputToken, outputToken);
            }
        }

The attacker can use a malicious path, for example, USDC,fee,MyToken,WETH. The decoded last token will be WETH, thus the check in _validateData will be passed. However, the UniswapV3 will decode this path as USDC,fee,MyToken and ignore the WETH when executing the swap.

The attacker can create a USDC-MyToken pool and swap all his locked USDC to MyToken and then remove all liquidity of this pool. As a result, the attacker's USDC will not be converted to the lpETH.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using UniswapV3's Path library(https://github.com/Uniswap/v3-periphery/blob/main/contracts/libraries/Path.sol#L29-L54) to decode the last token:

 using Path for bytes;
 ...
 ...
 uint256 pools = path.numPools();
 address last;
 for(uint256 i=0;i<pools;i++){
 (, address tokenOut,) = path.decodeFirstPool();
 last = tokenOut;
 }
 return last;

Assessed type

en/de-code

0xd4n1el commented 4 months ago

By using Claimed event we track if user indeed withdraw and adjust points accordingly. In this case the contract does not need further modifications

koolexcrypto commented 3 months ago

The issue doesn’t elaborate on the impact on the protocol.

However, as stated by the sponsor, Claimed event is emitted to track the claimed amounts.

        claimedAmount = address(this).balance;
        .
        .
        emit Claimed(msg.sender, _token, claimedAmount);

claimedAmount is ETH balance, so there is no points gained for the attacker in this case.

c4-judge commented 3 months ago

koolexcrypto marked the issue as primary issue