Munchables - Drynooo's results

Lines of code

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

Vulnerability details

Impact

Users may be attacked and unable to unlock assets.

Proof of Concept

Anyone can call the lockOnBehalf function to create a lock for someone else. This will extend the lockRecipient lock time. As long as the attacker repeatedly locks a user for a long time, the user will not be able to withdraw his assets. The attacker can set the lock amount to 0/1wei, which makes the attack almost cost-free.

Tools Used

manual

Recommended Mitigation Steps

It is recommended to set a minimum number of locks, or only allow migrationManager to call the lockOnBehalf function.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Users will lose locked funds.

Proof of Concept

If migrationManager calls the lock function, the logic in the if will not be executed. Therefore, the remainder amount will still be equal to 0, and subsequent updates to the remainder amount and quantity will cause the user to lose the previous amount of remainder. Indicates that the remainder deposited by the user cannot be divisible, and also belongs to the user's assets.

            if (msg.sender != address(migrationManager)) {
                // calculate number of nfts
                remainder = quantity % configuredToken.nftCost;
                numberNFTs = (quantity - remainder) / configuredToken.nftCost;

                if (numberNFTs > type(uint16).max) revert TooManyNFTsError();

                // Tell nftOverlord that the player has new unopened Munchables
                nftOverlord.addReveal(_lockRecipient, uint16(numberNFTs));
            }

Tools Used

manual

Recommended Mitigation Steps

It is recommended that the remainder variable should be calculated correctly regardless of whether the addReveal function is executed or not.

Assessed type

Other