sakshamguruji - Incorrect topUp Mechanism

[sherlock-admin]

sakshamguruji

medium

Incorrect topUp Mechanism

Summary

The topUp mechanism is incorrect and leads to topping up of staking contracts that are not the ones in the indices array.

Vulnerability Detail

The top up mechanism is simple , the executor passes a source address which would transfer the asset (rewardToken) to the respective staking contracts , the staking contracts are fetched from the stakingContracts array via the indices array , i.e. an indices array is passed in the function which holds the indices in the stakingContracts which are to be topped up

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

function topUp(
        address source,
        uint256[] memory indices
    ) external onlyRole(EXECUTOR_ROLE) {

From L258 the for loop starts and goes till indices.length and the staking reward contract is fetched as follows ->

StakingRewards staking = stakingContracts[i];

This is incorrect , let's take an example ->

1.) Assume the indices array was of length 5 and held the values 4,6,8,9,2

2.) The logic would read values stakingContracts[0] , stakingContracts[1] , stakingContracts[2] , stakingContracts[3] stakingContracts[4] (since i starts from 0 and increments till length -1) instead of reading the values stakingContracts[4] , stakingContracts[6] , stakingContracts[8] , stakingContracts[9] , stakingContracts[2]

And then the logic goes on to top up the incorrect staking contracts(L267).

Instead of reading StakingRewards staking = stakingContracts[i]; , it should read `StakingRewards staking = stakingContracts[indices[i]];

Impact

Almost all the time the incorrect staking contracts would be topped up and it would be impossible to correct this behavior unless all of the staking contracts are being topped up.

Code Snippet

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

Tool used

Manual review

Recommendation

Instead of StakingRewards staking = stakingContracts[i]; use `StakingRewards staking = stakingContracts[indices[i]];

Duplicate of #16

[sherlock-admin2]

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

takarez commented:

valid because { This is valid and a dupp of 016 issue with minimal impact}