in function setLockPeriods, multiplier can be set to lower than 100

[code423n4]

Handle

Tomio

Vulnerability details

Impact

in function setLockPeriods multiplier can be set to lower than 100 which will break the calculation when dividing the multiplier in function lock https://github.com/XDeFi-tech/xdefi-distribution/blob/master/contracts/XDEFIDistribution.sol#L268. If the amount times bonus multiplier below 100 the units value will be 0, therefore the totalUnits won't be added but the positionOf[tokenId] bill be added.

Proof of Concept

https://github.com/XDeFi-tech/xdefi-distribution/blob/master/contracts/XDEFIDistribution.sol#L77
https://github.com/XDeFi-tech/xdefi-distribution/blob/master/contracts/XDEFIDistribution.sol#L268

Tools Used

Recommended Mitigation Steps

in function setLockPeriods need to be add

        uint256 count = durations_.length;

        for (uint256 i; i < count; ++i) {
            require(multipliers >= 100); //added
            uint256 duration = durations_[i];
            require(duration <= uint256(18250 days), "INVALID_DURATION");
            emit LockPeriodSet(duration, bonusMultiplierOf[duration] = multipliers[i]);
        }
    }

[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 that the resulting units from the calculation 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.

[deluca-mike]

As alluded to above, bonusMultiplier is allowed to be below 100, and amount_ is no longer validated, and instead, resulting units are validated.

[Ivshti]

valid finding