sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

almurhasan - Council members will claim 0 telcoin tokens as tokenid burn mechanism is wrong. #158

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

almurhasan

high

Council members will claim 0 telcoin tokens as tokenid burn mechanism is wrong.

Summary

When a tokenid is burned, the tokenid is not replaced properly with the last tokenid (balances array) that leads to claim no telcoin tokens to some council members.

Vulnerability Detail

  1. Let assume 10 tokenid(council members) are minted. Every (0 to 5) tokenid’s balance is 50e18 and every (6 to 9) tokenid’s balance is 100e18.
  2. Now GOVERNANCE_COUNCIL_ROLE wants to burn tokenid 4, so GOVERNANCE_COUNCIL_ROLE calls the function burn with param tokenid 4.
  3. See the burn function, uint256 balance = balances[9] = 100e18; balances[4] = 100(balance of tokenid 4 is updated to 100 and this is not replaced with last one); balances.pop() will remove tokenid 9 and then tokenid 4 is burned.
  4. Tokenid 9 owner call the claim function to claim telcoin tokens but this will revert (see claim function) as balances[tokenId] = 0(as there is no tokenindex/tokenid 9 in balances array, so default value is 0); so balances[tokenId] -= amount i.e (0 - amount) = underflow ,which ledas to revert.

Impact

Council member can’t claim the telcoin tokens.

Code Snippet

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

Tool used

Manual Review

Recommendation

Replace the burning tokenid properly with the last tokenid.

Duplicate of #199

sherlock-admin2 commented 6 months ago

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

takarez commented:

valid because { valid as the watson demostrated the exploit scenario but with no adequate recommendations }