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

3 stars 1 forks source link

Users can be prevented fron unlocking their tokens through grieving attack #140

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

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

Vulnerability details

Impact

Users are unable to unlock their tokens during the period of the attack.

Proof of Concept

The function lockOnBehalf() is used to lock tokens for another useras it takes in the address of the user to lock tokens for which it uses to call the function lock() which then proceeds to lock the tokens for the lockrecipient, but this mechanism can be exploited to prevent the user(lockrecipient) from being able to unlock their tokens after their set duration is elapsed. This is because when locking tokens in the LockManager.sol, the duration is reset to it initial value which basically means that the duration for unlocking has been extended for said user. performing this attack is very easy on the side of the attacker as locking zero tokens for another user is possible due to lack of zero token amount check and also inexpensive as L2 does not use much gas on transactions, so the malicious actor can perform this attack indefinitely.

function lockOnBehalf(
        address _tokenContract,
        uint256 _quantity,
        address _onBehalfOf
    )
        external
        payable
        notPaused
        onlyActiveToken(_tokenContract)
        onlyConfiguredToken(_tokenContract)
        nonReentrant
    {
        address tokenOwner = msg.sender;
        address lockRecipient = msg.sender;
        if (_onBehalfOf != address(0)) {
            lockRecipient = _onBehalfOf;
        }

        _lock(_tokenContract, _quantity, tokenOwner, lockRecipient);
    }

Tools Used

Manual review

Recommended Mitigation Steps

input a signature verification mechanism that enables the user to sign and the signature verified before the transaction is executed.

Assessed type

Other

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory