Owner can DoS `MasterChef` rewards

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L86-L101

Vulnerability details

Impact

Owner can DoS all MasterChef rewards by adding a new token with very high _allocationPoints parameter, for example for totalAllocPoint to reach uint(-1). Then, any reward call for other token will give zero reward if earlier operated multipliers weren't close to uint(-1) (which will probably be the case). Any call to reward on the just added token will revert as reward calculation overflows (multiplying a number larger than 1 by uint(-1)). Thus the owner can stop all rewards to arrive, this can be done by mistake or as a deliberate action. Note that this cannot be undone.

Tools Used

Manual analysis

Recommended Mitigation Steps

Limit possible _allocationPoints to fit withing a large enough range to provide precision, but small enough not to cause overflow in any scenario.

Note that despite this mitigation, current mechanics enables the owner to inflate rewards for other tokens. This model may need to be reviewed.

[r2moon]

The reward will be allocated by owner, so owner won't DoS reward.

[GalloDaSballo]

I disagree with the sponsor that the owner won't DOS just because "trust".

Personally I think the finding has merit in that the math can be made to break, but ultimately this would have pretty much zero impact as people could just withdraw.

Also the likelihood of the sponsor doing this accidentally is pretty much zero as they would need to setup more rewards than the atom in the universe.

For these reasons I believe non-critical to be more appropriate

[JeeberC4]

Adding to warden's QA Report #241