The distribution of Telcoin among council members was not done correctly.

Summary

Telcoin is continuously distributing to council members, but the implementation of this logic is not correct.

Vulnerability Detail

When minting a new NFT, we insert a value of 0 into the balances array. This indicates that the council member initially has 0 Telcoins. There is a strict 1:1 mapping between the balances array and minted NFTs.

function mint(address newMember) {
    balances.push(0);
    _mint(newMember, totalSupply());
}

The owner of an NFT can claim the Telcoins assigned to them, and in doing so, the corresponding balances value should be reduced.

function claim(uint256 tokenId, uint256 amount) external {
    balances[tokenId] -= amount;
    TELCOIN.safeTransfer(_msgSender(), amount);
}

Telcoins are currently being distributed among the existing council members.

function _retrieve() internal {
    for (uint i = 0; i < balances.length; i++) {
        balances[i] += individualBalance;
    }
}

There is a logic error in the token burning process. When deleting the tokenId token, the current implementation removes the last value from the balances array.

function burn(uint256 tokenId, address recipient) {
    uint256 balance = balances[balances.length - 1];
    balances[tokenId] = balance;
    balances.pop();
    _burn(tokenId);
}

Let's consider an example with 5 council members. User A owns the 2nd NFT, and User B owns the 5th (last) NFT, with the balances array being [20, 22, 15, 15, 15].

When attempting to burn the 2nd NFT, the resulting balances array would be [20, 15, 15, 15]. This situation means that User B cannot claim their Telcoins because balances[4] is now 0. Additionally, User A cannot claim the remaining 15 Telcoins because he is no longer the owner of the 2nd NFT (which has already been burnt).

Impact

This issue will impact the accurate distribution of Telcoins among council members.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L180-L181 https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L292-L294 https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L108-L110 https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L218-L221

Tool used

Manual Review

Recommendation

Instead of popping the last value from the balances array when deleting an NFT, it is recommended to simply replace the value for the deleted NFT with 0.

Duplicate of #199

sherlock-audit / 2024-01-telcoin-judging

ggg_ttt_hhh - The distribution of Telcoin among council members was not done correctly. #179