search

sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Bauer - The last NFT owner may be unable to claim the reward #194

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

Bauer

high

The last NFT owner may be unable to claim the reward

Summary

The deletion in the burn function removes the balances[balances.length - 1] element, causing the owner of this NFT to be unable to claim the reward.

Vulnerability Detail

In the burn() function, the protocol first retrieves the value of the last element in the balances array. It then assigns this value to balances[tokenId], deletes the last element from balances, and finally burns the NFT with the specified ID. 

    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);
    }

The problem is that the element being deleted is balances.length - 1, which is not necessarily the same as the NFT ID (tokenId). As a result, when an owner attempts to claim from balances.length - 1, it will fail because balances[tokenId] has been set to 0.

Impact

The owner of the NFT with ID balances.length - 1 will be unable to claim the reward.

Code Snippet

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

Tool used

Manual Review

Recommendation

Duplicate of #199

sherlock-admin2 commented 5 months ago

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

takarez commented:

valid because { Valid because of the pact that the owner of the token thats last in the array will not be able to claim his reward due to the flaw in burn function; dupp of 109 also}