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

4 stars 4 forks source link

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

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#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

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #18

c4-judge commented 3 months ago

koolexcrypto changed the severity to 3 (High Risk)

c4-judge commented 3 months ago

koolexcrypto changed the severity to 2 (Med Risk)

c4-judge commented 3 months ago

koolexcrypto marked the issue as partial-75

c4-judge commented 3 months ago

koolexcrypto changed the severity to 3 (High Risk)

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 partial-25