sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

iberry - TelcoinDistributor: recoverERC20() can be used as a backdoor by the owner to retrieve Token, the owner can rug token #191

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

iberry

high

TelcoinDistributor: recoverERC20() can be used as a backdoor by the owner to retrieve Token, the owner can rug token

Summary

"The recoverERC20 function poses a risk of the owner retrieving both legitimate and potentially malicious tokens, leading to a 'rug pull' scenario where all tokens could be retrieved if the owner acts maliciously. This concern is also present in the StakingRewardsManager:recoverERC20() function. However, it's noted that in the CouncilMember:erc20Rescue scenario, the right to execute this function is restricted to the SUPPORT_ROLE."

Vulnerability Detail

The recoverERC20() function in TelcoinDistributor currently provides the owner with the privilege to retrieve tokens, posing a concern. This issue manifests in three different parts.

Impact

medium, rug token

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/protocol/core/TelcoinDistributor.sol#L223-L228 https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/telx/core/StakingRewardsManager.sol#L230-L237 https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L355-L361

Tool used

Manual Review

Recommendation

Access Control: Implement stringent access control mechanisms, such as role-based access, to limit the execution of recoverERC20() to authorized users or roles. This helps mitigate the risk of unauthorized access, particularly by the owner.Limit possibilities of recoverERC20()

Audit and Transparency: Establish an audit trail or logging system to monitor calls to recoverERC20(). This ensures transparency and facilitates the detection and investigation of any suspicious activities. Add emit ERC20Recovered(token, msg.sender, amount);

Multi-Signature Wallet: Explore the use of a multi-signature wallet for recoverERC20(). This adds an extra layer of security by requiring multiple signatures or approvals before any token retrieval is permitted.

sherlock-admin2 commented 8 months ago

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

takarez commented:

invalid because { invalid; owners are trusted}

nevillehuang commented 8 months ago

Invalid, admins are trusted entities trusted not to be malicious.