sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

grearlake - Latest council member will lost rewards after burning council member NFT #248

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

grearlake

high

Latest council member will lost rewards after burning council member NFT

Summary

After burning council member NFT, latest user in council member list will lost rewards due to updating of balances array

Vulnerability Detail

Function burn() is used to burn council member NFT:

function burn(
    uint256 tokenId,
    address recipient
) external onlyRole(GOVERNANCE_COUNCIL_ROLE) {
    require(totalSupply() > 1, "CouncilMember: must maintain council");
    _retrieve();
    _withdrawAll(recipient, tokenId);

    uint256 balance = balances[balances.length - 1];
    balances[tokenId] = balance;
    balances.pop();
    _burn(tokenId);
}

Consider scenario: 1, User A, B, C, D are council members and have tokenId= 0, 1, 2, 3. 2, User B's NFT is burned, balance of user D, which is balances[3] is set to balances[1]. 3, User D are not able to claim rewards because there is no token left in balances[3].

Impact

Latest user in council member list will lost rewards

Code Snippet

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

Tool used

Manual Review

Recommendation

Do not change sort of balances when burning. When distribute tokens to council member, do not distribute rewards to balances location that being burned

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 {valid and a dupp of 109}