Lines of code

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

Vulnerability details

Impact

When user lock tokens, and execute calculation count of nfts, some value could be write into remainder value. But after unlockTime, user can call unlock() and get all tokens back, but remainder value does not delete, so in next call to lock() function, he could use less tokens, than it needed, because will be uses specified quantity and remainder value from previous lock.

Proof of Concept

Example: configuredToken.nftCost = 3; User call lock(10) -> quantity=10, remainder=1 (10%3=1)

function lock() {
   ....
   remainder = quantity % configuredToken.nftCost;
   numberNFTs = (quantity - remainder) / configuredToken.nftCost;
   ...
   lockedToken.remainder = remainder; // 1
   lockedToken.quantity += _quantity; // 3

When block.timestamp will be > unlockTime, user call unlock(10).

function unlock() {
   ...
   lockedToken.quantity -= _quantity;
   // But dont set 0 in remainder value.

User call lock(8)

function lock() {
   ...
   uint256 quantity = _quantity + lockedToken.remainder;  // 8 + 1= 9
   ...
   remainder = quantity % configuredToken.nftCost; // 9%3=0
   numberNFTs = (quantity - remainder) / configuredToken.nftCost; // (9-0) / 3=3

So, user use less tokens in second call of lock, than in first call.

Tools Used

Manual review

Recommended Mitigation Steps

   function unlock() {
   ...
   lockedToken.quantity -= _quantity;
+  lockedToken.remainder = 0;

Assessed type

Other

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

When user unlock all tokens, remainder value does not delete #441