Existing council member can become member twice

Summary

Already existing council member can become a member twice as there is no check if a address is an existing member of council or not

Vulnerability Detail

New council member can be added through mint & removeFromOffice but both the functions are lacking check on that if address is already an existing council member, If a council member became a member twice then that member will receive TELCOIN balance twice which is bad as other members will loss balance

   function removeFromOffice(
        address from,
        address to,
        uint256 tokenId,
        address rewardRecipient
    ) external onlyRole(GOVERNANCE_COUNCIL_ROLE) {
        // Retrieve and distribute any pending TELCOIN for all council members
        _retrieve();
        // Withdraw all the TELCOIN rewards for the specified token to the rewardRecipient
        _withdrawAll(rewardRecipient, tokenId);
        // Transfer the token (representing the council membership) from one address to another
        _transfer(from, to, tokenId);
    }

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

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

Impact

User can became council member twice & if that happens, that member will receive TELCOIN twice

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Use a mapping or array to store council member address and add a checks in mint & removeFromOffice function or can also check if a address is holding any member NFT then should not became member again

sherlock-admin2 commented 5 months ago

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

takarez commented:

invalid because { both this functions are admin control and will make sure they are not twice; according to sherlock they are trusted}

nevillehuang commented 5 months ago

Invalid, this would consititute user/admin input error not valid based on sherlock rules, see point 5.

sherlock-audit / 2024-01-telcoin-judging

araj - Existing council member can become member twice #217