Munchables - 0rpse's results

Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L274-L294 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L382-L384 https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L256-L261

Vulnerability details

Impact

Users are able to push unlockTime of other users' locks further by locking dust amounts of tokens using lockOnBehalf.

Proof of Concept

The function lockOnBehalf lets users lock tokens on other's behalf. This will increase the unlockTime of the token locked:

        lockedToken.unlockTime =
            uint32(block.timestamp) +
            uint32(_lockDuration);

As there are no minimum amount checks, an attacker could use dust amounts to easily affect unlockTime. At this point the only way out for the funds would be setting the lock duration to a very small value, this is because if it is set to zero funds will be locked for another 30 days(minLockDuration), also note setLockDuration will not let a user change his/her duration if it is decreasing the unlockTime:

                if (
                    uint32(block.timestamp) + uint32(_duration) <
                    lockedTokens[msg.sender][tokenContract].unlockTime
                ) {
                    revert LockDurationReducedError();
                }

Example: User locks some tokens for 30 days at time t.

At the end of lock or a day before attacker locks on user's behalf.

Now the user will have to wait for an extra 30 days to withdraw these funds.

In order to prevent this a user would have to repeatedly call setLockDuration to decrease its lock duration at many intervals. Otherwise attacker can keep locking to push unlockTime further.

Tools Used

Manual review

Recommended Mitigation Steps

You could introduce permission functionality for locking on other's behalf and/or add minimum amount checks for lockOnBehalf so that the attack would be expensive.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

A user can reduce a lockup time that was previously set, breaking one of the invariants of the protocol. This could for example be used to harvest bonus undeserved schnibbles.

Proof of Concept

Proof of Concept The function setLockDuration resets the lock duration for every token. While doing this, it uses the newly set duration plus lastLockTime, which is the last point in time when a token was locked.

The issue arises because setLockDuration does not update the token's lastLockTime. As a result, after the initial locking, a user can set setLockDuration to a bigger value, harvest rewards for a longer lock, and then use setLockDuration again with the stale lastLockTime to reduce the unlock time and receive undeserved rewards.

Example: A user locks some tokens at time t with a lock duration of 30 days.

After 20 days, the user calls setLockDuration and sets it to 60 days.

At this point, unlockTime is correctly set to t + 60, with the lock duration being 60 days.

Now, block.timestamp = t + 20 and lastLockTime = t. This allows the user to set the lock duration to 40 days since the following check passes:

                if (
                    uint32(block.timestamp) + uint32(_duration) <
                    lockedTokens[msg.sender][tokenContract].unlockTime
                ) {
                    revert LockDurationReducedError();
                }

The unlockTime is then set with the stale lastLockTime:

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

This sets unlockTime to t + 40 days, decreasing the lock time that was previously set to t + 60.

Tools Used

Manual review

Recommended Mitigation Steps

setLockDuration should set lastLockTime to block.timestamp after setting unlockTime.

Assessed type

Other