User tokens could freeze by frontrunners until the next emergency mode on if any

[c4-bot-3]

Lines of code

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

Vulnerability details

User tokens could freeze by frontrunners until the next emergency mode on

Impact

when alice who locked tokens wants to withdraw her tokens in the final moments of emergency mode, there is a possible frontrun attack by frontrunners which can change the order of the transactions between Alice's withdrawal and the admin who turned the emergency mode back to off. after the startClaimDate if the admin turn on the emergency mode, if alice can't withdraw her tokens in this time mode, then she will never can until the next emergency mode. the impact is that the tokens of users could freeze by frontrunners until the next emergency mode on

function withdraw(address _token) external {
        if (!emergencyMode) {
            if (block.timestamp <= loopActivation) {
                revert CurrentlyNotPossible();
            }
            if (block.timestamp >= startClaimDate) {
                revert NoLongerPossible();
            }
        }
        // ...
}

  function setEmergencyMode(bool _mode) external onlyAuthorized {
        emergencyMode = _mode;
    }

Proof of Concept

Here is the scenario:

1. emergency mode is on after the startClaimDate. In the final moment of emergency mode, alice wants to use the opportunity and withdraw her tokens and calls the withdraw function
2. Alice's transaction goes to mempool waiting to be executed (let's call it TX Alpha)
3. after that the admin calls the setEmergencyMode(false) and turn off the emergency mode. this transaction also goes to mempool (let's call it TX Beta)
4. if first Alpha then Beta then there is no problem but the frontrunner sees the opportunity and switches the order of these two transactions meaning instead of first Alpha then Beta, it goes Beta then Alpha, leading to revert for alice with NoLongerPossible error cause now the emergency mode is off and the block.timestamp >= startClaimDate alice can't withdraw her tokens anymore until the next emergency mode if any.

put this test into the PrelaunchPoints.t.sol:

    function test_RevertNoLongerPossible_WithdrawAfterConvertEmergencyMode_Frontrunned() public {
        address alice = makeAddr("alice");
        uint256 lockAmount = 100 ether;
        vm.startPrank(alice);
        lrt.mint(alice, lockAmount);
        lrt.approve(address(prelaunchPoints), lockAmount);
        prelaunchPoints.lock(address(lrt), lockAmount, referral);
        vm.stopPrank();
        uint256 balanceBefore = lrt.balanceOf(alice);

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

        prelaunchPoints.setEmergencyMode(true);

        prelaunchPoints.setEmergencyMode(false);    // TX 2 (Beta)
        vm.prank(alice);
        prelaunchPoints.withdraw(address(lrt));     // TX 1 (Alpha)

        assertEq(prelaunchPoints.balances(alice, address(lrt)), 0);
        assertEq(lrt.balanceOf(alice) - balanceBefore, lockAmount);

    }

switch the TX 2 and TX 1 to see how the results differ when the frontrunner switches TX orders.

Tools Used

manual review

Recommended Mitigation Steps

create a space for users to request a withdrawal by creating a request ID instead of a withdrawing operation and Let the admin call a new authorized function that loops over each request ID and withdraws their tokens. in this case, you don't need any time restriction to withdraw. users request a withdrawal operation and at the correct time, the admin himself does the operation after that users get their tokens the new function could be something like this:

    function executeWithdraw(uint256 requestIds[], uint256 maxIteration) public onlyAuthorized {
        uint i = 0;
        while(i < maxIteration /* to avoid DOS */) {
            uint id = requestIds[i];
            i += 1;
            uint256 lockedAmount = withdrawRequest[id].lockedAmount
            balances[withdrawRequest[id].owner][withdrawRequest[id].token] = 0;
            if (withdrawRequest[id].token == ETH){
                // ...
            } else {
                // ...
            }
        }
    }

[c4-bot-3]

Discord id(s) for hunter(s): [object Object]

[bytes032]

requires consequence of admin actions, this submission might work for an audit but not for a bug bounty

nothing directly exploitable rn