sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

fibonacci - `StakingRewardsManager`: incorrect `StakingRewards` contracts top up #115

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

fibonacci

medium

StakingRewardsManager: incorrect StakingRewards contracts top up

Summary

The topUp function uses incorrect indices to retrieve a StakingRewards contract for top up.

Vulnerability Detail

This function receives an array of staking indices that need to be top upped. It then iterates over these indices. However, instead of using the value from this array, it uses the value of the current iteration of the loop.

Therefore, if, for instance, an array with indices [3, 7] is provided, stakings with indices [0, 1] will be mistakenly top upped.

Impact

Unexpected StakingRewards contracts may be top upped.

Code Snippet

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

Tool used

Manual Review

Recommendation

diff --git a/telcoin-audit/contracts/telx/core/StakingRewardsManager.sol b/telcoin-audit/contracts/telx/core/StakingRewardsManager.sol
index ea3853c..3435898 100644
--- a/telcoin-audit/contracts/telx/core/StakingRewardsManager.sol
+++ b/telcoin-audit/contracts/telx/core/StakingRewardsManager.sol
@@ -257,7 +257,7 @@ contract StakingRewardsManager is AccessControlUpgradeable {
     ) external onlyRole(EXECUTOR_ROLE) {
         for (uint i = 0; i < indices.length; i++) {
             // get staking contract and config
-            StakingRewards staking = stakingContracts[i];
+            StakingRewards staking = stakingContracts[indices[i]];
             StakingConfig memory config = stakingConfigs[staking];

             // will revert if block.timestamp <= periodFinish

Duplicate of #16

sherlock-admin2 commented 9 months ago

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

takarez commented:

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

takarez commented:

invalid because { This is invalid because the failed function is an indication that there is something wrong with inputed index thus allowing the caller to put the right va;ue; and this is not DOS btw}