sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Arz - Minting a new council member NFT will not work after burning one #107

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

Arz

medium

Minting a new council member NFT will not work after burning one

Summary

When a Council member NFT is burned and its id is not the last, minting new Council member NFTs will not work.

Vulnerability Detail

When the admin burns a Council member NFT this decreases the totalSupply and if the token that was burned wasnt the token with the last id, minting new tokens will not work.

For example we have 3 tokens and the first one is being burned. The totalSupply is now 2, the admin then decides that he wants to mint a new council member NFT so he calls mint() which will call _mint(newMember, 2); but because the token with id 2 already exists this will cause the transaction to revert and the admin wont be able to mint a new NFT.

Impact

The admin will fail to add new council members because the mint() function will always revert.

Code Snippet

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

_mint(newMember, totalSupply());

As you can see totalSupply() is being used when minting, this will be a problem after a token is burned and the supply decreases.

Tool used

Manual Review

Recommendation

One way to fix this would be to burn the last token id, and transfer the token that is being burned to the owner of the last token id. Because the last token id is burned, the admin will then be able to mint it again.

Duplicate of #199

sherlock-admin2 commented 7 months ago

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

takarez commented:

valid because { this is valid with but a dupp of 14 as they are of the same underlying cause}