If the diffNumSeconds is lower than curEpochLen, it return false.
Also, if the diffNumSeconds is more than MAX_EPOCH_LENGTH, it return false.
Normal user or malicious one could set the curEpochLen to the MAX_EPOCH_LENGTH. And it means that he must call the checkpoint function at the exact second in the time, which could be problematic. Since the block.timestamp is updated every 15 sec on Mainnet, and in the range of 800 milisec to 3 sec on L2's. So, in such case i believe the user would fail to update the checkpoint forever. Since
Proof of Concept
Assume:
MAX_EPOCH_LENGTH = 31449600
curEpochLen = 31449600
It means that user who calls checkpoint() should wait till such point in time, when diffNumSeconds returns the number that must be not less 31449600 and simultaneously not more than 31449600, which i believe is very problematic.
Impact
The user/owner would fail to call the checkpoint(). The only option is to use some kind of automation script, which i believe not the case.
Recommendation
Adjust the check that it still serves the Olas purpose, but simultaneously doesn’t impose so strickt checks.
Lines of code
https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/Tokenomics.sol#L1102-L1103
Vulnerability details
Vulnerability Detail
Assume,
curEpochLen
= MAX_EPOCH_LENGTH, to be precise it is 31449600 seconds.When the
checkpoint()
is invoked, it takesdiffNumSeconds
.The
diffNumSeconds
return the delta of the end of last epoch with the current timestamp.After that, the
currentEpochLen
is taken.Then, the function makes a crucial checks, which determines whether we could proceed or not.
diffNumSeconds
is lower thancurEpochLen
, it return false.diffNumSeconds
is more thanMAX_EPOCH_LENGTH
, it return false.Normal user or malicious one could set the
curEpochLen
to theMAX_EPOCH_LENGTH
. And it means that he must call thecheckpoint
function at the exact second in the time, which could be problematic. Since theblock.timestamp
is updated every 15 sec on Mainnet, and in the range of 800 milisec to 3 sec on L2's. So, in such case i believe the user would fail to update the checkpoint forever. SinceProof of Concept
Assume:
MAX_EPOCH_LENGTH
= 31449600curEpochLen
= 31449600It means that user who calls
checkpoint()
should wait till such point in time, whendiffNumSeconds
returns the number that must be not less 31449600 and simultaneously not more than 31449600, which i believe is very problematic.Impact
The user/owner would fail to call the
checkpoint()
. The only option is to use some kind of automation script, which i believe not the case.Recommendation
Adjust the check that it still serves the Olas purpose, but simultaneously doesn’t impose so strickt checks.
Assessed type
Math