sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

ggg_ttt_hhh - Council members have the ability to transfer their NFTs to others. #195

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

ggg_ttt_hhh

medium

Council members have the ability to transfer their NFTs to others.

Summary

The creation of new council members should be community-driven, but the current implementation does not correctly reflect this.

Vulnerability Detail

There is a disparity in permission control for NFT transfers. While the removeFromOffice function requires the governance role to be called, the transferFrom andsafeTransferFrom functions work without any permission checks. This allows council members to easily send their NFTs using these functions.

function removeFromOffice(address from, address to, uint256 tokenId, address rewardRecipient) 
external onlyRole(GOVERNANCE_COUNCIL_ROLE) {
    _retrieve();
    _withdrawAll(rewardRecipient, tokenId);
    _transfer(from, to, tokenId);
}

Impact

Code Snippet

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

Tool used

Manual Review

Recommendation

Overwrite these functions.

Duplicate of #243

sherlock-admin2 commented 6 months ago

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

takarez commented:

valid because {This is a meduim severity finding and a dupp of 190 issue}