Munchables - 0xMosh's results

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L382

Vulnerability details

Impact

Anyone can increse/grief users unlock time repeatedly by locking dust amounts .

Proof of Concept

lockOnBehalf functionality is for locoking on behalf of other users . Calling this function with a valid user as recipient will result in a increase in user's unlocktime equal to his lockduration . And there are no minimum threshold for the locking amount(_quantity ) .

 function lockOnBehalf(
        address _tokenContract,
        uint256 _quantity,
        address _onBehalfOf
    )
        //..
    {
        address tokenOwner = msg.sender;
        address lockRecipient = msg.sender;
        if (_onBehalfOf != address(0)) {
            lockRecipient = _onBehalfOf;
        }
        _lock(_tokenContract, _quantity, tokenOwner, lockRecipient);
    }

 function _lock(
        address _tokenContract,
        uint256 _quantity,
        address _tokenOwner,
        address _lockRecipient
    ) private {
       //..
        lockedToken.quantity += _quantity;
        lockedToken.lastLockTime = uint32(block.timestamp);
@>          lockedToken.unlockTime =//@audit-issue 
            uint32(block.timestamp) +
            uint32(_lockDuration);

This can be used against user by locking dust amount of tokens on behalf of the victim . As there are no threshold regarding minimum locking amount ,locking dust amount of token will prolong user's unlocktime . And user will fail to unlock his funds as intended .And repeatedly doing this users fund can be locked for very long time . Because of the low gas cost on Blast Chain , this exploit can be pulled of in very low cost .

Tools Used

Manual review

Recommended Mitigation Steps

Two mitigation is possible :

1. Implement a minimum locking amount check .(Doesnot mitigate the issue completely )
2. Have a functionality where user can enable/disable " behalf of" locking on his position .

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L257

Vulnerability details

Impact

funds can be unlocked before unlock time !

Proof of Concept

setLockDuration function is for updating the lock duration of a user's locked fund . However one issue arises when a user set a token lock duration less then his previous lock duration . The reason behind this is token unlock time is being updated as lastLockTime + _duration .

  function setLockDuration(uint256 _duration) external notPaused {
       //..
        for (uint256 i; i < configuredTokensLength; i++) {
            address tokenContract = configuredTokenContracts[i];
            if (lockedTokens[msg.sender][tokenContract].quantity > 0) {
                // check they are not setting lock time before current unlocktime
                if (
                    uint32(block.timestamp) + uint32(_duration) <
                    lockedTokens[msg.sender][tokenContract].unlockTime
                ) {
                    revert LockDurationReducedError();
                }

                uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
                    .lastLockTime;
                lockedTokens[msg.sender][tokenContract].unlockTime =
@>                  lastLockTime +
                    uint32(_duration);
            }
        }

Consider this scenario :

1. Alice locked his tokens for 60 days by calling lock . Now , his lastLockTime = block.timestamp & unlockTime = 60 days later .
2. after passing 31 days , alice sets his lock duration to 30 . This passes the check at line 257 . Now ,his updated unlock time is lastLockTime + _duration which is 30 days prior to actual unlockTime of 60 days !
3. Alice now can unlock his tokens .

Tools Used

Manual review

Recommended Mitigation Steps

Setting a new duration should not impact the previous locks . Remove this logic :

 lockedTokens[msg.sender][tokenContract].unlockTime =
                    lastLockTime +
                    uint32(_duration);

Assessed type

Context