sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

ggg_ttt_hhh - There is a misuse of indices input in the topUp function. #239

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

ggg_ttt_hhh

medium

There is a misuse of indices input in the topUp function.

Summary

Vulnerability Detail

In the StakingRewardsManager, there is a stakingContracts array containing all the stakingRewards contracts managed by this contract. The topUp function is used to assign rewards to specificstakingRewards contracts, but there is a misuse of indices input value.

function topUp(address source, uint256[] memory indices) external onlyRole(EXECUTOR_ROLE) {
    for (uint i = 0; i < indices.length; i++) {
        StakingRewards staking = stakingContracts[i];          <== @audit       we should use indices array here
        StakingConfig memory config = stakingConfigs[staking];
    }
}

Currently, the function uses the index i instead of indices[i]. In order to send rewards to the last stakingRewards contract, it's necessary to include all stakingRewards contracts as input and validate the configuration file accordingly.

A notable consideration is the transfer of ownership by the admin for certain stakingRewards. In such cases, the manager might be restricted from calling the notifyRewardAmount function of those stakingRewards. Consequently, it becomes challenging to notify rewards for stakingRewards contracts that appear later in the stakingContracts array after the one with the ownership transfer.

Impact

Sending rewards to specific stakingRewards contracts poses a challenge.

Code Snippet

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

Tool used

Manual Review

Recommendation

function topUp(address source, uint256[] memory indices) external onlyRole(EXECUTOR_ROLE) {
    for (uint i = 0; i < indices.length; i++) {
-        StakingRewards staking = stakingContracts[i];  
+        StakingRewards staking = stakingContracts[indices[i]];  
    }
}

Duplicate of #16

sherlock-admin2 commented 5 months ago

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

takarez commented:

valid because { This is valid because the watson explain how incorrect use of indices poses a challenge of depositing into the intended contract and also a dupp of 016}