RewardsPool.inflate sets wrong value to RewardsPool.InflationIntervalStartTime variable

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/RewardsPool.sol#L98

Vulnerability details

Impact

RewardsPool.inflate sets wrong value to RewardsPool.InflationIntervalStartTime variable. As result inflation logic will be broken and GGP token will inflate much faster than it was expected.

Proof of Concept

GGP token has limited total supply that is minted on deploy.
But at the beginning not all tokens are available. Token amount should inflate during the time to distribute its total supply.

To increase amount of tokens in circulation RewardsPool.startRewardsCycle function is called.
This function then calls inflate function.

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/RewardsPool.sol#L82-L100

	function inflate() internal {
		ProtocolDAO dao = ProtocolDAO(getContractAddress("ProtocolDAO"));
		uint256 inflationIntervalElapsedSeconds = (block.timestamp - getInflationIntervalStartTime());
		(uint256 currentTotalSupply, uint256 newTotalSupply) = getInflationAmt();


		TokenGGP ggp = TokenGGP(getContractAddress("TokenGGP"));
		if (newTotalSupply > ggp.totalSupply()) {
			revert MaximumTokensReached();
		}


		uint256 newTokens = newTotalSupply - currentTotalSupply;


		emit GGPInflated(newTokens);


		dao.setTotalGGPCirculatingSupply(newTotalSupply);


		addUint(keccak256("RewardsPool.InflationIntervalStartTime"), inflationIntervalElapsedSeconds);
		setUint(keccak256("RewardsPool.RewardsCycleTotalAmt"), newTokens);
	}

This function calculates how many periods has passed since last inflate and then increases token's total supply.
It uses getInflationIntervalStartTime function to know when inflation was done last time. getInflationIntervalStartTime function just checks InflationIntervalStartTime variable.

Later inflate function is updating InflationIntervalStartTime variable to provide time when last inflation was done.
addUint(keccak256("RewardsPool.InflationIntervalStartTime"), inflationIntervalElapsedSeconds);
The problem is that inflationIntervalElapsedSeconds is set instead of block.timestamp.
inflationIntervalElapsedSeconds is a time in seconds between the last inflation was done. It's not a timestamp.
As a result when inflate will be called next time, then getInflationAmt function will calculate new tokens incorrectly and will distribute all total supply more faster.

Example.
1.RewardsPool is deployed at time 1000. Then InflationIntervalStartTime == 1000.
2.Inflation interval is 10.
3.inflate is called for first time at time 1010. As result price has increased once. InflationIntervalStartTime == inflationIntervalElapsedSeconds == 10.
4.inflate is called next time at time 1020. inflationIntervalElapsedSeconds = 1020 - 10 = 1010. And price has changed 101 times instead of 1 as getInflationAmt function decided that 101 period has passed since last inflation.

Tools Used

VsCode

Recommended Mitigation Steps

Set block.timestamp to inflationIntervalElapsedSeconds variable.

[GalloDaSballo]

Should be dup of #648 but something is off, will double check and keep separate for now

[0xju1ie]

I think the warden is confused. The InflationIntervalStartTime is added to, not set.

According to the code their example should be the following:
Example.
1.RewardsPool is deployed at time 1000. Then InflationIntervalStartTime == 1000.
2.Inflation interval is 10.
3.inflate is called for first time at time 1010. As result price has increased once. inflationIntervalElapsedSeconds == 10, InflationIntervalStartTime == 1010 .
4.inflate is called next time at time 1020. inflationIntervalElapsedSeconds = 1020 - 1010 = 10.

[GalloDaSballo]

I believe that despite the grammar mistake, the warden has found a dup of #648

Because they didn't show the end conclusion, am awarding half

[c4-judge]

GalloDaSballo marked the issue as duplicate of #648

[c4-judge]

GalloDaSballo marked the issue as partial-50

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)