GetInflationAmount
can run out of gas
#477
Labels
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-710
edited-by-warden
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Lines of code
https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/RewardsPool.sol#L74
Vulnerability details
Impact
The loop within
getInflationAmount
can run out of gas, irreversibly breaking the contractProof of Concept
Currently, the function
startRewardsCycle
callsinflate
which then callsgetInflationAmount
in order to receive the inflated supply. However, during the functiongetInflationAmount
it loops over allinflationIntervalsElapsed
, which is in most cases 28 days, when called regularly. However, ifstartRewardsCycle
is not called regularly, this means thatinflationIntervalStartTime
is not updated regularly:addUint(keccak256("RewardsPool.InflationIntervalStartTime"), inflationIntervalElapsedSeconds);
this will then eventually lead to to a DoS where the loop runs out of gasfor (uint256 i = 0; i < inflationIntervalsElapsed; i++) {
due to too many elapsed inflation intervals.Tools Used
VSCode
Recommended Mitigation Steps
Currently, there is no easy solution. One could implement pagination for the loop to prevent this issue, however, this would need a larger logic change. Therefore our recommendation is to a) keep an eye on the duration time and b) eventually use chainlink automation to call
startRewardsCycle
every 28 days.Eventually, one could also set an upper limit of
inflationIntervalsElapsed
and if this limit is exceeded, simply exceed e.g. 50 loops.https://docs.chain.link/chainlink-automation/introduction
*This is the first contest i took part, therefore my submissions might not be ideal yet, if there are any questions feel free to contact me directly in discord :-)
** While this issue initially was assessed as
HIGH
, we think thatMEDIUM
is here more appropriate because we do not see this happening in production (it would take quite some weeks of forgetting to call the function), however, the code itself is still vulnerable for this issue and in our opinion this is what counts.The text was updated successfully, but these errors were encountered: