New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inflation rate can be reduce by half at most if it get called every 1.99 interval. #648
Comments
Dupe #811 |
Primary for now due to better explanation |
GalloDaSballo marked the issue as primary issue |
emersoncloud marked the issue as disagree with severity |
I agree with this issue but assets can't be stolen, lost or compromised directly. Medium severity is more appropriate https://docs.code4rena.com/awarding/judging-criteria#estimating-risk |
I have considered a Higher Severity, due to logical flaws. However, I believe that the finding
For those reasons, I believe Medium Severity to be the most appropriate |
GalloDaSballo changed the severity to 2 (Med Risk) |
Acknowledged, not fixing in this first version of the protocol. We can and will have rialto call startRewardsCycle if needed, and think it's unlikely to become delayed. |
GalloDaSballo marked the issue as selected for report |
Lines of code
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/RewardsPool.sol#L98
Vulnerability details
Impact
When doing inflation, function
getInflationAmt()
calculated number of intervals elapsed by dividing the duration with interval length.As we can noticed that, this calculation is rounding down, it means if
block.timestamp - startTime = 1.99 intervals
, it only account for1 interval
.However, when updating start time after inflating, it still update to current timestamp while it should only increased by
intervalLength * intervalsElapsed
instead.Since default value of inflation interval = 1 days and reward cycle length = 14 days, so the impact is reduced. However, these configs can be changed in the future.
Proof of Concept
Consider the scenario:
InflationIntervalStartTime = 100
.InflationIntervalSeconds = 50
.timestamp = 199
, functiongetInflationAmt()
will calculateinflate()
function,InflationIntervalStartTime
is still updated to current timestamp, soInflationIntervalStartTime = 199
.While at
timestamp = 595
, inflated times should be(595 - 100) / 50 = 9
instead.Tools Used
Manual Review
Recommended Mitigation Steps
Consider only increasing
InflationIntervalStartTime
by the amount of intervals time interval length.The text was updated successfully, but these errors were encountered: