CouncilMember::burn() function is incorrectly implemented
Summary
CouncilMember::burn() function is incorrectly implemented as it updates the balance for a token that is being currently burnt.
Vulnerability Detail
In the burn function, the Telcoin balances for all the councilMembers are updated wit retrieve function call. Immediately after that, the TelCoin for the tokenId is withdrawn to the recipient address.
This clears the balances of the tokenId that is being burnt.
So, what is left is to update the balances array by removing the tokenId from the array and then burning the token itself.
Refer to the below code snippet, where the above intention is implemented. Instead of remove the tokenId from the array, the tokenId balance is updated with the balance of the last element in the array.
using Array is not a good approach here. It is recommended to use map with tokenId -> balance.
This way, deletion will be much more easier and simpler.
ravikiran.web3
high
CouncilMember::burn() function is incorrectly implemented
Summary
CouncilMember::burn() function is incorrectly implemented as it updates the balance for a token that is being currently burnt.
Vulnerability Detail
In the burn function, the Telcoin balances for all the councilMembers are updated wit retrieve function call. Immediately after that, the TelCoin for the tokenId is withdrawn to the recipient address.
This clears the balances of the tokenId that is being burnt. So, what is left is to update the balances array by removing the tokenId from the array and then burning the token itself.
Refer to the below code snippet, where the above intention is implemented. Instead of remove the tokenId from the array, the tokenId balance is updated with the balance of the last element in the array.
Impact
Code Snippet
https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L210-L222
Tool used
Manual Review
Recommendation
using Array is not a good approach here. It is recommended to use map with tokenId -> balance. This way, deletion will be much more easier and simpler.
Duplicate of #199