ravikiran.web3 - StakingRewardManager::setStakingRewardsFactory() will break the sync of RewardContracts maintained between manager and factory

[sherlock-admin2]

ravikiran.web3

medium

StakingRewardManager::setStakingRewardsFactory() will break the sync of RewardContracts maintained between manager and factory

Summary

setStakingRewardsFactory will break the parallel copies maintained between manager and factory. Also, the intention to maintain parallel copies is not clear.

Vulnerability Detail

Depending on how the StakingRewards contracts are referred in the protocol, the results returned could be very different.

When a new StakingReward contract is out by the manager contract, the copy of the StakingRewards contract is updated in the factory as well as manager contracts.

But, when admin update the staking factory with a new instance, the StakingRewards data is not the same between the two contract.

Impact

The data between factory and manager contract will be different and will effect the working of the protocol if both were used interchangeably.

Code Snippet

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

function setStakingRewardsFactory(
        StakingRewardsFactory factory
    ) external onlyRole(MAINTAINER_ROLE) {
        //check for zero values
        require(
            address(factory) != address(0),
            "StakingRewardsManager: Factory cannot be set to zero"
        );
        //set new value
        stakingRewardsFactory = factory;
        emit StakingRewardsFactoryChanged(factory);
    }

Tool used

Manual Review

Recommendation

As long as the whole protocol refers the manager contract for stakingRewards, it is safe to use it.

[sherlock-admin2]

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

takarez commented:

invalid because { the watson did not provide a clear procedure of how the issue may occur }

[nevillehuang]

Invalid, setStakingRewardsFactory() is an admin only function. This issue also does not prove how changing the factory contract will cause any fund loss/dos impact.