Munchables - niser93's results

Lines of code

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

Vulnerability details

Impact

An attacker can lock his/her funds for a victim player using the lockOnBehalf() method. However, this action triggers the _lock() method which reset the lockedToken.unlockTime. Because there is no minimum quantity to lock, an attacker can reset the lockedToken.unlockTime of a victim indefinitely. This is a DoS attack on the unlock functionality of the victim.

Proof of Concept

The lockOnBehalf() method allows to lock own funds for another address:

LockManager.sol#L274-L294

274      /// @inheritdoc ILockManager
275      function lockOnBehalf(
276          address _tokenContract,
277          uint256 _quantity,
278          address _onBehalfOf
279      )
280          external
281          payable
282          notPaused
283          onlyActiveToken(_tokenContract)
284          onlyConfiguredToken(_tokenContract)
285          nonReentrant
286      {
287          address tokenOwner = msg.sender;
288          address lockRecipient = msg.sender;
289          if (_onBehalfOf != address(0)) {
290              lockRecipient = _onBehalfOf;
291          }
292  
293          _lock(_tokenContract, _quantity, tokenOwner, lockRecipient);
294      }

This method resets the lockedToken.unlockTime of the lockRecipient address. Even if the attacker locks 1 wei on behalf of lockRecipient, the lockedToken.unlockTime is set again at least to lockdrop.minLockDuration:

LockManager.sol#L382-L384

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

So, an attacker can postpone the unlockTime of a victim indefinitely, because the victim will not be able to call the unlock method successfully (line 410):

LockManager.sol#L400-L411

400      /// @inheritdoc ILockManager
401      function unlock(
402          address _tokenContract,
403          uint256 _quantity
404      ) external notPaused nonReentrant {
405          LockedToken storage lockedToken = lockedTokens[msg.sender][
406              _tokenContract
407          ];
408          if (lockedToken.quantity < _quantity)
409              revert InsufficientLockAmountError();
410          if (lockedToken.unlockTime > uint32(block.timestamp))
411              revert TokenStillLockedError();

Tools Used

Visual inspection

Recommended Mitigation Steps

Only a pre-authorized address should be able to lock funds on behalf of another address. So, we suggest to introduce a mapping to track pre-authorized addresses and add/remove methods to manage them.

Assessed type

DoS