Lines of code

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

Vulnerability details

Impact

In getPoolReward, all windows of the pool are iterated to calculate the reward. When lastPoolRewardTime < window.startTime, the reward rate of the previous window is got. When the user claims the reward for the first time, lastPoolRewardTime == 0. If pool.rewardWindows[0].startTime > 0, since lastPoolRewardTime < window.startTime, it will get pool.rewardWindows[0-1].reward, at this point an overflow will occur, resulting in the user not being able to claim the reward.

            uint256 totalReward;
            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;

pool.rewardWindows.startTime is configured in configurePools, but configurePools does not require pool.rewardWindows[0].startTime == 0, if pool.rewardWindows[0].startTime > 0, getPoolReward will fail and the user will not be able to claim the reward.

    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; ) {
                _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; }
        }
    }

Proof of Concept

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

Tools Used

None

Recommended Mitigation Steps

Change to

            uint256 totalReward;
            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) {
+               if (lastPoolRewardTime < window.startTime && i != 0) {
                    uint256 currentRewardRate = pool.rewardWindows[i - 1].reward;

code-423n4 / 2023-03-neotokyo-findings