StakingRewardManager::topUp() implementation is buggy

Summary

The implementation of topUp function in StakingRewardManager contract is incorrect and could result in funding other StakingReward contracts.

Vulnerability Detail

Refer to the below code snippet, where in topUp accepts an array of indices for which additional rewards are moved to the staking contract.

 function topUp(
        address source,
        uint256[] memory indices
    ) external onlyRole(EXECUTOR_ROLE) {
        for (uint i = 0; i < indices.length; i++) {
            // get staking contract and config
            StakingRewards staking = stakingContracts[i];
            StakingConfig memory config = stakingConfigs[staking];

The intention was to pass indices as below to topUp the staking Rewards contracts at position 0, 2, 3 and 10 based on the state variable stakingContracts of the StakingRewardManager contract.

example : indices[0,2,3,10]

But, instead of referring the index from the indices, the logic is looping through the length of the indices and uses incrementing variable as index for fetching staking Reward contract to top up.

While the intention was to topUp staking Reward contracts at position 0,2 ,3 and 10, the logic actually topUp the staking Reward contracts at 0,1,2,3.

Hence potentially funding totally different staking Reward contract.

Impact

TopUp() function adds funds to a totally different stakingReward contracts and not inline with the intention of the caller of the function. The logic itself is wrongly implemented.

Code Snippet

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

Tool used

Manual Review

Recommendation

Revise the implementation as below.

  function topUp(
        address source,
        uint256[] memory indices
    ) external onlyRole(EXECUTOR_ROLE) {
        for (uint i = 0; i < indices.length; i++) {
            // get staking contract and config
       @audit ===>    uint256 stakeIndex = indices[i];
       @audit ===>    require(stakeIndex < stakingContracts.length,"Invalid Indices");

       @audit ===>    StakingRewards staking = stakingContracts[stakeIndex];
            StakingConfig memory config = stakingConfigs[staking];

Duplicate of #16

sherlock-audit / 2024-01-telcoin-judging

ravikiran.web3 - StakingRewardManager::topUp() implementation is buggy #160