code-423n4 / 2023-03-neotokyo-findings

4 stars 0 forks source link

Error in the calculation of `daoShare` value in the `getPoolReward` function #420

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1389

Vulnerability details

Impact

In the getPoolReward function inside the NeoTokyoStaker contract there is an error in the calculation of the daoShare value which represents the part of rewards sent to the treasury, this means that the treasury will receive less rewards that what it is supposed to.

Proof of Concept

The error occurs in the code below :

File: NeoTokyoStaker.sol Line 1388-1392

uint256 share = points * _PRECISION / pool.totalPoints * totalReward;
uint256 daoShare = share * pool.daoTax / (100 * _DIVISOR);
share /= _PRECISION;
daoShare /= _PRECISION;
return ((share - daoShare), daoShare);

As you can see the first line calculates the user shares amount which is multiplied by the constant _PRECISION to avoid rounding errors, and in the second line the daoShare value is calculated this value represent a percentage of shares sent to the treasury and thus its formula should be :

uint256 daoShare = (share * pool.daoTax) / _DIVISOR;

Where the value of pool.daoTax is a percentage set in basis point of _DIVISOR == 100 (meaning that 100 ≈ 100%).

But there is an error in the actual formula used in the code as it contains a division by both _DIVISOR and 100 which is wrong because the resultant daoShare value will be very small and will not represent the correct percentage set by the protocol.

And thus the final value returned by the function will be wrong as it will give more rewards to the user and less to the treasury which was not intended.

Tools Used

Manual review

Recommended Mitigation Steps

Correct the formula used to calculate the daoShare value and replace it by :

uint256 daoShare = (share * pool.daoTax) / _DIVISOR;
hansfriese commented 1 year ago

daoTax is in bps so should divide by 10000.

c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Invalid