sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

ggg_ttt_hhh - There is an issue in the logic when minting a new NFT. #189

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

ggg_ttt_hhh

high

There is an issue in the logic when minting a new NFT.

Summary

Vulnerability Detail

When burning an already minted token, the associated token ID is removed.

function burn(uint256 tokenId, address recipient) {
    _burn(tokenId);
}

Subsequently, when minting a new token, the newly created token ID should be set to the current total supply.

function mint(address newMember) {
    _mint(newMember, totalSupply());
}

There is a notable logic error in this scenario. For instance, if we have already minted 5 tokens with IDs [0, 1, 2, 3, 4], and then we delete the token with ID 1, the current total supply becomes 4. Now, when attempting to create a new council member, the logic tries to mint a new NFT with token ID 4. However, token ID 4 has already been minted, resulting in the failure of any attempts to mint a new NFT.

Impact

Once any NFT is deleted, it becomes impossible to add a new council member.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L221 https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L181

Tool used

Manual Review

Recommendation

The newly created token ID should not be the current total supply. Instead, it is advisable to store this value separately to avoid potential issues.

Duplicate of #199

sherlock-admin2 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

vlaid because {This is valid because it has the same underlying cause as the issue 109}