psb01 - Incorrect implementation of burn() causes losses and undefined behaviour.

[sherlock-admin2]

psb01

high

Incorrect implementation of burn() causes losses and undefined behaviour.

Summary

Wrongly implemented burn() function causing loss to the balances of other Token IDs and also causing unintended behaviour.

Vulnerability Detail

CouncilMember.sol Inside burn() function before burning tokenId It calls _withdrawAll() which set balances[tokenId] = 0 then balances[tokenId] value is again set equivalent to balances[lastTokenId] and and then array is popped causing loss to balances[lastTokenId] value.

Impact

Suppose There are tokenIds from 0 to 10 and burn(5,recipient) is called so after execution of this function balances[5] = balances[10], balances array will be popped also tokenId = 5 will be burned causing loss of balances value corresponding to tokenId = 10 now if again burn(10,recipient) is called but balances[10] will not exist causing Unintended behaviour.

Code Snippet

CouncilMember.sol

Tool used

Manual Review

Recommendation

Implement burn() function correctly.

Duplicate of #199

[sherlock-admin2]

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

takarez commented:

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