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

Invariant is broken, one ETH deposit by user is not convertable to one lpETH, if someone manually deposits eth to contract

howlbot-integration opened this issue · 7 comments

Lines of code

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

Vulnerability details

Impact

Within the context of invariants, it's emphasized that when a user deposits 1 ETH, it should automatically convert to 1 lpETH. However, if an individual chooses to directly send ETH to the contract, this disrupts the conversion mechanism, diverging from the user's intended perspective.

Proof of Concept

By incorporating the provided test into the existing test file, it becomes evident that the conversion process is compromised.

function testBreakOneToOneConversion(uint256 lockAmount) public {
        lockAmount = bound(lockAmount, 1, 1e36);
        vm.deal(address(this), lockAmount);
        prelaunchPoints.lockETH{value: lockAmount}(referral);

        // Directly deposit 1 eth
        address userDepositETH = makeAddr("user");
        vm.deal(address(userDepositETH), 1e18);
        vm.prank(userDepositETH);
        (bool sent, bytes memory data) = address(prelaunchPoints).call{
            value: 1e18
        }("");
        vm.stopPrank();

        // 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);
        prelaunchPoints.claim(
            ETH,
            100,
            PrelaunchPoints.Exchange.UniswapV3,
            emptydata
        );

        uint256 balanceLpETH = (prelaunchPoints.totalLpETH() * lockAmount) /
            prelaunchPoints.totalSupply();

        assertEq(prelaunchPoints.balances(address(this), ETH), 0);
        assertEq(lpETH.balanceOf(address(this)), balanceLpETH);
        assertEq(lpETH.balanceOf(address(this)), lockAmount);
    }

Tools Used

Manual review

Recommended Mitigation Steps

To resolve this issue, removing the receive function can be an effective solution. The receive function can be found in the contract code here.

Assessed type

Other

koolexcrypto marked the issue as duplicate of #18

koolexcrypto changed the severity to 3 (High Risk)

koolexcrypto changed the severity to 2 (Med Risk)

koolexcrypto marked the issue as partial-75

koolexcrypto changed the severity to 3 (High Risk)

koolexcrypto marked the issue as duplicate of #33

koolexcrypto marked the issue as partial-25