Lines of code

https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/interfaces/ILockManager.sol#L22-L27

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/interfaces/ILockManager.sol#L22-L27

    struct ConfiguredToken {
        uint256 usdPrice;
        uint256 nftCost;
        uint8 decimals;
        bool active;
    }

Each configured token has an implementation of this struct where the nftCost is the cost of the NFT associated with locking this token and the usdPrice is the USD price per token, issue however is that going to the implementation of the we can see that there is only a functionality to update the price of the underlying token. However, no functionality whatsoever exists that indicates that the nft cost associated to locking the token could be updated, which breaks the logic of NFTs would be key to note that an NFT does not have a permanent cost and could also depreciate/appreciate with time.

Impact

With time after the initial locking of the tokens there would be a disparity between the real cost of the NFT and what it's become now, which would cause the over/undervaluation of the NFT, depending on whether the NFT appreciated/depreciated.

This would then make the amount of NFTs calculated below to wrong since with time after the first locking, it uses a heavily stale data https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L361-L370

            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));
            }

Thereby inflating/deflating the amount of unrevealed the Munchable NFTs the user has, since the data that's being sent to the NFTOverlord#addReveal() would be wrong.

Recommended Mitigation Steps

Introduce a functionality to update the nftCost.

Assessed type

Context

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