setLockPeriods accepts multipliers less than 100 (1x mult)

[code423n4]

Handle

StErMi

Vulnerability details

Impact

In the setLockPeriods you are not making any checks on the multipliers value for a given index. It means that the multiplier could be less than 100 (1x).

Proof of Concept

Tools Used

Manual

Recommended Mitigation Steps

If you don't want to enable multiplier less than 100 add a check on setLockPeriods like

require(multipliers[i] >= 100, "INVALID_MULTIPLIER");

[deluca-mike]

Supporting less that 100 (i.e. less than 1x) was intended behaviour (i.e. 30-day lockup gets you normal rewards, and, say 0-day lockups gets you 0.5x rewards). However, there is a good catch since the resulting units from the calculation in lock should be checked for 0 (instead of checking the amount_ argument for 0), since that's a better and more all-encompassing way to filter out amount that are too low (or zero) after taking the multiplier into account. This is low-risk though, because the amount would have to be miniscule (less than 100 wei of XDEFI) for 0 units to result.

[deluca-mike]

Duplicate #96

[Ivshti]

@deluca-mike tbh I am on the fence between this being an invalid finding and your assessment of it as low-risk

Under 1 multipliers are a very useful tool as you say, and really small multiplier leading to a 0 amount, leaving behind some dust also sounds ok