sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Bauer - Minting to users using `totalSupply()` as the NFT ID is incorrect #172

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

Bauer

high

Minting to users using totalSupply() as the NFT ID is incorrect

Summary

In the mint() function, the protocol uses totalSupply() as the NFT ID to mint to the user. However, during a burn, totalSupply() decreases, causing the wrong NFT ID to be minted to the user.

Vulnerability Detail

In the mint() function, the protocol mints an NFT with the latest value of totalSupply() as the NFT ID and assigns it to the newMember. However, during a burn operation, where totalSupply() decreases, it results in transferring an NFT with an owner to another person, subsequently transferring the associated rewards as well.

    function mint(
        address newMember
    ) external onlyRole(GOVERNANCE_COUNCIL_ROLE) {
        if (totalSupply() != 0) {
            _retrieve();
        }

        balances.push(0);
        _mint(newMember, totalSupply());
    }

Impact

Transferring an existing NFT to another person also results in transferring the associated rewards to that person.

Code Snippet

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

Tool used

Manual Review

Recommendation

Adding a new variable to record the quantity of minted NFTs.

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 { this is not like others in terms of the impact and the flaw scenario but the underlying cause is the same thus making it a dupp of 109}