cryptofish7
commented
2 years ago Should be a 2.
Also disputed as in the fraction (rJoePerSec * 1e18) / joeSupply, the numerator is always bigger than denominator.
Max supply of JOE is 500mm, scaled to 1e18 so max value in wei of joeSupply is 1e(18+8) = 1e26.
We always scale rJoePerSec to 1e18 here, so numerator is 1e(18+18) = 1e36.
Therefore numerator > denominator and there should be no round to zero error.
dmvt
Handle
moose-code
Vulnerability details
Impact
Any user can very simple get the accRJoePerShare value to stay constant in the staker contract. The consequence is obvious. No one can accrue rJOE and hence no rJOE available for users to participate in launch pools.
Proof of Concept
Okay here we go. It all has to do with the public function updatePool() and how accRJoePerShare is calculated.
accRJoePerShare = accRJoePerShare + (rJoeReward * PRECISION) / joeSupply;
Where accRJoePerShare = accRJoePerShare + (multiplier rJoePerSec PRECISION) / joeSupply;
And therefore: accRJoePerShare = accRJoePerShare + ( (block.timestamp - lastRewardTimestamp) rJoePerSec PRECISION) / joeSupply;
Now ignore the first term in this calc (accRJoePerShare) as this will stay constant, we are interested in how accRJoePerShare increases and therefore: ( (block.timestamp - lastRewardTimestamp) rJoePerSec PRECISION) / joeSupply;
Given block times on avalanche are fast, frequently time between blocks is 1 second, this function can be called every block, and therefore we can assume that (block.timestamp - lastRewardTimestamp) is often equal to around 1 if an attacker would like (or a small constant below 10.
Therefore our equation now simplifies to (given PRECISION is set as 1e18 in contract): (rJoePerSec * 1e18) / joeSupply;
Okay! So now the issue. If (rJoePerSec * 1e18) < joeSupply Then this number becomes zero (no decimals in solidity obviously) and accRJoePerShare is constant.
Now is definitely possible as there is no limit to the amount of joeSupply in the contract (besides total supply of JOE). If everyone stakes there JOE, its very likely this will be the case! (rJoePerSec * 1e18) < joeSupply
There are no bounds or checks on the rJoePerSec and PRECISION value!
In order to avoid this problem its important that at least rJoePerSec * PRECISION > joeSupply and here we should consider joeSupply as the total amount of JOE in circulation!
But there are no checks at all on this equality or on PRECISION and rJoePerSec! Its likely that if the joeSupplies is great, and the rJoePerSec accumulated is low, that zero accumulation will happen.
Tools Used
Manual inspection and coffee.
Recommended Mitigation Steps
There need to be strict checks on this function function updateEmissionRate(uint256 _rJoePerSec) external onlyOwner { updatePool(); rJoePerSec = _rJoePerSec; emit UpdateEmissionRate(msg.sender, _rJoePerSec); }
That take into account the value that PRECISION is set to as well as the total existing JOE supply