An inactive pool breaks staking functionality across all other pools

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/dfa5887062e47e2d0c801ef33062d44c09f6f36e/contracts/staking/NeoTokyoStaker.sol#L1319-L1320

Vulnerability details

Impact

When a pool is inactive, all non-admin interactions with the staking contract will fail.

Proof of Concept

Within the stake function of NeoTokyoStaker.sol, the following check exists...

    // Validate that the asset being staked matches an active pool.
    if (_pools[_assetType].rewardWindows[0].startTime >= block.timestamp) {
        revert InactivePool(uint256(_assetType));
    }

This implies that individual pools can be configured to become active at a future time.

However, this is incompatible with how the getPoolReward function operates. Due to the following logic...

    if (lastPoolRewardTime < window.startTime) {
        uint256 currentRewardRate = pool.rewardWindows[i - 1].reward;

The initial state for a users lastPoolRewardTime is 0.

When users first interact with any pool, getPoolReward is called on every existing pool, meaning that if any pool is inactive, on the first pass through the loop, window.startTime will be greater than lastPoolRewardTime.

Logic will flow into the if block, and i - 1 will be 0 - 1, which will underflow and revert.

Therefore interactions with every pool will be broken until the inactive pool(s) becomes active.

Tools Used

Manual review.

Recommended Mitigation Steps

Either require that the first window of all pools starts at 0, allowing the removal of the check in the stake function.

Or, adjust the logic in the getPoolReward function to handle the case where the first reward window of a pool could have a future startTime.

[c4-judge]

hansfriese marked the issue as satisfactory

[c4-judge]

hansfriese marked the issue as duplicate of #280

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-b