Council members can transfer their NFTs to someone else

Summary

Council Members can transfer their NFTs although they are not supposed to.

Vulnerability Detail

Protocol mentions:

There will be governance votes in which the councel members are elected. This is why they cannot transfer their own NFTs. only the election process can switch them.

Council members can transfer their own NFTs by calling the transferFrom function in ERC721 since the function is not overriden in CouncilMember.sol.

ERC721.sol
     * @dev See {IERC721-transferFrom}.
     */
    function transferFrom(address from, address to, uint256 tokenId) public virtual override {
        //solhint-disable-next-line max-line-length
        require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved");

        _transfer(from, to, tokenId);
    }

    /**
     * @dev See {IERC721-safeTransferFrom}.
     */
    function safeTransferFrom(address from, address to, uint256 tokenId) public virtual override {
        safeTransferFrom(from, to, tokenId, "");
    }

    /**
     * @dev See {IERC721-safeTransferFrom}.
     */
    function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual override {
        require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved");
        _safeTransfer(from, to, tokenId, data);
    }

The call to _transfer will call the _beforeTokenTransfer() hook, which will call ERC721Enumerable.sol, which will update the index.

ERC721Enumerable.sol
_beforeTokenTransfer()
        if (from == address(0)) {
            _addTokenToAllTokensEnumeration(tokenId);
        } else if (from != to) {
            _removeTokenFromOwnerEnumeration(from, tokenId);
        }
        if (to == address(0)) {
            _removeTokenFromAllTokensEnumeration(tokenId);
        } else if (to != from) {
            _addTokenToOwnerEnumeration(to, tokenId);

Impact

Council members can transfer tokens to another address, which can affect the proposal, challenge and execution part of the protocol in Telcion Distributor.sol.

Council members that are about to get their tokenId burned can transfer their token to another address, bypassing the burn.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin-cryptostaker2/blob/475e5c92315d0b6cc1aa185a856fb8c24d993040/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L371-L378

Tool used

Manual Review

Recommendation

Recommend overriding and reverting all the transferFrom functions in ERC721 so that the council member cannot transfer tokens to another address.

Only CouncilMember.sol can directly call the _transfer() internal function in ERC721.sol.

Duplicate of #243

sherlock-audit / 2024-01-telcoin-judging

Jaraxxus - Council members can transfer their NFTs to someone else #190