sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

dipp - Incorrect balance is removed when burning #234

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

dipp

high

Incorrect balance is removed when burning

Summary

The balance of the last tokenId minted is removed when burning any token.

Vulnerability Detail

When minting a new council member nft, the balances array is increased and the tokenId of the nft is directly tied to the index of the balances array. When a token is burned the balance of the tokenId is set to the last balance in balances and the last balance is removed, decreasing the length of the balances array.

When the owner of the last token wants to claim, an index out of bounds error will be thrown since the balances array was reduced and the index/tokenId is no longer valid.

Impact

Council members may lose their allocated TELCOINs if nfts are burned.

Code Snippet

CouncilMember.sol:mint#L173-L182

CouncilMember.sol:burn#L210-L222

CouncilMember.sol:claim#L92-L111

Tool used

Manual Review

Recommendation

Balances should not be removed, the length must be the same as the totalSupply. Since _withdrawAll is called in burn, the balance of the removed token is already set to 0 so removing from balances is not necessary. Alternatively, consider using a mapping instead of an array to keep track of balances.

Duplicate of #199

sherlock-admin2 commented 8 months ago

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

takarez commented:

valid because { This is valid and a dupp of 109 with minimal impact}