code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

Invalid stake level thresholds can break `InfinityStaker.getUserStakeLevel` logic #347

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L351-L361 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L213-L223

Vulnerability details

Impact

The current staking level (voting power) for a given user is figured out by the function InfinityStaker.getUserStakeLevel based on defined thresholds.

However, the internal logic assumes the various levels to have their values defined in ascending order (BRONZE_STAKE_THRESHOLD < SILVER_STAKE_THRESHOLD < GOLD_STAKE_THRESHOLD < PLATINUM_STAKE_THRESHOLD).

Impact: Users have a wrong staking level, which results in incorrect voting power.

Proof of Concept

If the staking level values are completely mixed up and for instance, BRONZE_STAKE_THRESHOLD = 6000 and SILVER_STAKE_THRESHOLD = 4000, then a user with a totalPower = 5000 will have the staking level NONE, however, it should be SILVER_STAKE_THRESHOLD.

InfinityStaker.updateStakeLevelThreshold

function updateStakeLevelThreshold(StakeLevel stakeLevel, uint16 threshold) external onlyOwner {
    if (stakeLevel == StakeLevel.BRONZE) {
        BRONZE_STAKE_THRESHOLD = threshold;
    } else if (stakeLevel == StakeLevel.SILVER) {
        SILVER_STAKE_THRESHOLD = threshold;
    } else if (stakeLevel == StakeLevel.GOLD) {
        GOLD_STAKE_THRESHOLD = threshold;
    } else if (stakeLevel == StakeLevel.PLATINUM) {
        PLATINUM_STAKE_THRESHOLD = threshold;
    }
}

InfinityStaker.sol#L213-L223

if (totalPower <= BRONZE_STAKE_THRESHOLD) {
    return StakeLevel.NONE;
} else if (totalPower > BRONZE_STAKE_THRESHOLD && totalPower <= SILVER_STAKE_THRESHOLD) {
    return StakeLevel.BRONZE;
} else if (totalPower > SILVER_STAKE_THRESHOLD && totalPower <= GOLD_STAKE_THRESHOLD) {
    return StakeLevel.SILVER;
} else if (totalPower > GOLD_STAKE_THRESHOLD && totalPower <= PLATINUM_STAKE_THRESHOLD) {
    return StakeLevel.GOLD;
} else {
    return StakeLevel.PLATINUM;
}

Tools Used

Manual review

Recommended mitigation steps

Consider adding a check to make sure the level values have the invariant BRONZE_STAKE_THRESHOLD < SILVER_STAKE_THRESHOLD < GOLD_STAKE_THRESHOLD < PLATINUM_STAKE_THRESHOLD enforced.

HardlyDifficult commented 2 years ago

Agree that it would be good to validate the inputs are sorted correctly here. Lowering risk and merging with the warden's QA report https://github.com/code-423n4/2022-06-infinity-findings/issues/345