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

4 stars 0 forks source link

Incorrect `window.startTime` can temporary cause DOS #414

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

While out of scope, it is important to note, that the function getPoolReward() which is used to calculate and mint rewards can only work if the pool.rewardWindows[0].startTime == 0 and will revert with index underflow otherwise:

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

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

The pool's window startTime is configured via configurePools() which allows to set the first window startTime to a positive value:

if (j != 0 && _inputs[i].rewardWindows[j].startTime <= lastTime) {
    revert RewardWindowTimesMustIncrease();
}
lastTime = _inputs[i].rewardWindows[j].startTime;

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

A positive value of the first window means that the pool's rewards should start from some specific timestamp and not from the begnning of time. This reasoning may be used by an admin to set the first window startTime to a positive value, thus breaking the contract temporarily until pools are reconfigured again.

Proof of Concept

x

Tools Used

x

Recommended Mitigation Steps

Add a requirement in configurePools() that the first window startTime should always be zero:

require(_inputs[i].rewardWindows[0].startTime == 0);
c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Out of scope

c4-judge commented 1 year ago

hansfriese marked the issue as satisfactory

c4-judge commented 1 year ago

hansfriese marked the issue as duplicate of #280

c4-judge commented 1 year ago

hansfriese changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

hansfriese marked the issue as grade-b