search

sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

eeshenggoh - Adding stake contract with different rewards tokens will cause tokens to be stuck #114

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

eeshenggoh

high

Adding stake contract with different rewards tokens will cause tokens to be stuck

Summary

In StakingRewardsManager.sol::addStakingRewardsContract() The function implementation does not have checks for rewardToken address.

Vulnerability Detail

It is stated in Natspec that This function WILL NOT REVERT if staking does not have the right rewardToken. However, no checks are done. This function is crucial because of StakingRewardsManager.sol::topUp().

The topUp() sends reward token to addresses that are added to the StakingRewardsManager contract. If the added contract has different ERC20 rewardTokens set up, this means when the EXECUTOR calls the topUp(), wrong rewards are sent, and that particular staking contract will not update the rewards ratio, and participants are not able to claim tokens.

Impact

1) rewardTokens are stuck in StakingRewards contract. 2) StakingRewards rewardRate will not be updated correctly as it uses the ERC20 different token address balanceOf function. 3) Participants are not able to claim the rewardTokens sent by manager.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/main/telcoin-audit/contracts/telx/core/StakingRewardsManager.sol#L128-L138

Tool used

Manual Review

Recommendation

Check if the stakingReward tokens used is the same as StakingRewardsManager

sherlock-admin2 commented 7 months ago

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

takarez commented:

invalid because { The funcion in question has an admin modifier which is considered trusted ;making this invalid}

goheesheng commented 7 months ago

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

takarez commented:

invalid because { The funcion in question has an admin modifier which is considered trusted ;making this invalid}

The staking contract specify the tokens staked and rewards to be distributed, and will transfer ownership to the stakingmanager contract after. After the transfer, the contract does not have the ability to modify the stake contracts. Which render the staking contract useless and the added contract without checks causing rewards to be distributed wasted

nevillehuang commented 7 months ago

Invalid, addStakingRewardsContract() is a permissioned function, where trusted admins are trusted to add appropriate token rewards