When advancing epoch, previousEpochTimestamp is set to block.timestamp, which may shift the emission schedule, makes accountability non deterministic, and is not coherent with the rest of the code.
Proof of Concept
The contract behaves differently between line 65 and line 73.
Assume epoch 1 last 100 seconds and started at timestamp 1000, then we'd expect the next epoch to start at timestamp 1100.
If no one calls advanceEpoch until timestamp 1201 then due to the line epochsPassedSinceLastAdvance = (block.timestamp - previousEpochTimestamp) / getEpochDuration() which will be 2, the next epoch will indeed have started at epoch 1100 and the tokens will be released. Then epoch 3 will start at timestamp 1201.
But if someone calls advanceEpoch at timestamp 1199, then due to previousEpochTimestamp = block.timestamp, the epoch 2 would only start at timestamp 1199, and timestamps between 1100 and 1199 wouldn't be in any epoch.
Therefore the behavior of the contract differs:
in one case the next epoch is considered to be starting right after the next one
in the other case it is considered to be starting when the function is called
This behavior may lead to accounting errors as you cannot know in advance the exact timestamps of epochs so how many tokens you have for a period, erratic behavior and eventually a loss in token emissions if advanceEpoch is called at a timestamp where it shifts the whole emission schedule.
Lines of code
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L73
Vulnerability details
Impact
When advancing epoch,
previousEpochTimestamp
is set toblock.timestamp
, which may shift the emission schedule, makes accountability non deterministic, and is not coherent with the rest of the code.Proof of Concept
The contract behaves differently between line 65 and line 73.
Assume epoch 1 last 100 seconds and started at timestamp 1000, then we'd expect the next epoch to start at timestamp 1100.
If no one calls
advanceEpoch
until timestamp 1201 then due to the lineepochsPassedSinceLastAdvance = (block.timestamp - previousEpochTimestamp) / getEpochDuration()
which will be 2, the next epoch will indeed have started at epoch 1100 and the tokens will be released. Then epoch 3 will start at timestamp 1201.But if someone calls
advanceEpoch
at timestamp 1199, then due topreviousEpochTimestamp = block.timestamp
, the epoch 2 would only start at timestamp 1199, and timestamps between 1100 and 1199 wouldn't be in any epoch.Therefore the behavior of the contract differs:
This behavior may lead to accounting errors as you cannot know in advance the exact timestamps of epochs so how many tokens you have for a period, erratic behavior and eventually a loss in token emissions if
advanceEpoch
is called at a timestamp where it shifts the whole emission schedule.Recommended Mitigation Steps
Change
previousEpochTimestamp = block.timestamp;
topreviousEpochTimestamp = previousEpochTimestamp + epochsPassedSinceLastAdvance * getEpochDuration();