Denial of Service when calling the `stake()` function when setting the `startTime` greater than zero

[code423n4]

Lines of code

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

Vulnerability details

Impact

Users will be unable to call the stake() function to stake assets in the pool. This affects the availability of the platform.

Proof of Concept

The stake() function will be reverted due to the underflow at line 1320 when the window.startTime of the first index is greater than 0, which is set in the configurePools() function by the authorized entity.

When the user calls stake() function for the first time, lastPoolRewardTime will be 0 and if the window.startTime > 0, the condition check in line 1319 will be passed, and access to the pool.rewardWindows[i - 1] array index will be reverted in the 0 iteration due to integer underflow.

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

uint256 lastPoolRewardTime = lastRewardTime[_recipient][_assetType];
uint256 windowCount = pool.rewardCount;
for (uint256 i; i < windowCount; ) {
    RewardWindow memory window = pool.rewardWindows[i];

    /*
        If the last reward time is less than the starting time of this 
        window, then the reward was accrued in the previous window.
    */
    if (lastPoolRewardTime < window.startTime) {
        uint256 currentRewardRate = pool.rewardWindows[i - 1].reward;

Tools Used

Visual Studio Code / Manual Review

Recommended Mitigation Steps

Adding the restriction when setting the window.startTime of the first index in the configurePools() function is not greater than 0.

function configurePools (
    PoolConfigurationInput[] memory _inputs
) hasValidPermit(UNIVERSAL, CONFIGURE_POOLS) external {
    for (uint256 i; i < _inputs.length; ) {
        uint256 poolRewardWindowCount = _inputs[i].rewardWindows.length;
        _pools[_inputs[i].assetType].rewardCount = poolRewardWindowCount;
        _pools[_inputs[i].assetType].daoTax = _inputs[i].daoTax;

        // Set the pool reward window details by populating the mapping.
        uint256 lastTime;
        for (uint256 j; j < poolRewardWindowCount; ) {
            ///////////// Mitigation Steps //////////////
            if(j==0 && _inputs[i].rewardWindows[j].startTime > 0){
              revert RewardWindowTimesMustLessThanZero();
            }
            /////////////////////////////////////////////
            _pools[_inputs[i].assetType].rewardWindows[j] =
                _inputs[i].rewardWindows[j];

            // Revert if an invalid pool configuration is supplied.
            if (j != 0 && _inputs[i].rewardWindows[j].startTime <= lastTime) {
                revert RewardWindowTimesMustIncrease();
            }
            lastTime = _inputs[i].rewardWindows[j].startTime;
            unchecked { j++; }
        }
        unchecked { ++i; }
    }
}

[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