search

sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

sakshamguruji - Council Member NFT Would Be Lost If New Council Member Contract Does Not Implement onERC721Received #214

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

sakshamguruji

medium

Council Member NFT Would Be Lost If New Council Member Contract Does Not Implement onERC721Received

Summary

Council Member NFT Would Be Lost If New Council Member Contract Does Not Implement onERC721Received , this is because a plain transfer is performed to transfer the council member nft to the receiver .

Vulnerability Detail

1.) The function removeFromOffice in the CouncilMember.sol contract replaces an existing council member with a new one and withdraws the old member's TELCOIN allocation

2.) It withdraws the balance of that tokenId and sends it to a receiver and then the NFT is sent to the to address at L133

_transfer(from, to, tokenId);

3.) This is a plain transfer , meaning if the to address (most probably a contract as confirmed by the sponsor) does not implement a onERC721Received then the NFT transfer would lock up the nft at the to address and would be impossible to retrieve.

Impact

Council Member NFT Would Be Lost If New Council Member Contract Does Not Implement onERC721Received

Code Snippet

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

Tool used

Manual Review

Recommendation

Instead of a plain transfer implement safeTransfer of nft

Duplicate of #25

sherlock-admin2 commented 5 months ago

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

takarez commented:

invalid because { invalid according sherlock rule VII. number 21}