__141345__ - Rewards might be lost due to rounding down error

[sherlock-admin]

141345

medium

Rewards might be lost due to rounding down error

Summary

When the total share is big _amount * POINTS_MULTIPLIER / shares could round down to 0, as a result, pointPerShare would remain the same but the reward _amount is transferred, this portion of rewards will be lost.

Vulnerability Detail

When the shares grow large enough compared to POINTS_MULTIPLIER, _amount * POINTS_MULTIPLIER / shares could round down to 0. Even not 0, if shares is just above half of POINTS_MULTIPLIER, the rounding error can still make the user receive in-proportional pointPerShare. In case _amount is 1, and shares is (type(uint128).max / 2 + 1), then, the resultant pointPerShare would be only half compared to the value it should be.

Impact

The user might lose the reward _amount, but receive 0 or in-proportional pointPerShare.

Code Snippet

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/BasePool.sol#L95-L98

https://github.com/sherlock-audit/2022-10-merit-circle/blob/main/merit-liquidity-mining/contracts/base/AbstractRewards.sol#L89-L99

Tool used

Manual Review

Recommendation

Add checks to make sure pointPerShare will be increased.

    require(_amount * POINTS_MULTIPLIER / shares > 0);

[jack-the-pug]

Invalid.

Just so you know, distributeRewards() is supposed to be called by the owner to add rewards, not by some end user. And if _amount * POINTS_MULTIPLIER / shares rounds down to 0, that means the owner should add more rewards.

Sending only 1 wei of rewards token and ending up not actually increasing the pointPerShare is expected.

[federava]

This behavior is part of the design and it is specified in the NatSpec of _distributeRewards