Summary
In the LinearDistributionIntervalDecrease::getPeriodReward
function there is a multiplication after division. In Solidity this can lead to incorrect result due to the rounding issue. The LinearDistributionIntervalDecrease::getPeriodReward
function is important for critical functions in the Distribution
contract.
Vulnerability Details
In the LinearDistributionIntervalDecrease::getPeriodReward
function is performed multiplication of the result of division:
function getPeriodReward(
uint256 initialAmount_,
uint256 decreaseAmount_,
uint128 payoutStart_,
uint128 interval_,
uint128 startTime_,
uint128 endTime_
) external pure returns (uint256) {
if (interval_ == 0) {
return 0;
}
if (startTime_ < payoutStart_) {
startTime_ = payoutStart_;
}
uint128 maxEndTime_ = _calculateMaxEndTime(payoutStart_, interval_, initialAmount_, decreaseAmount_);
if (endTime_ > maxEndTime_) {
endTime_ = maxEndTime_;
}
if (startTime_ >= endTime_) {
return 0;
}
uint256 timePassedBefore_ = startTime_ - payoutStart_;
if ((timePassedBefore_ / interval_) == ((endTime_ - payoutStart_) / interval_)) {
@> uint256 intervalsPassed_ = timePassedBefore_ / interval_;
@> uint256 intervalFullReward_ = initialAmount_ - intervalsPassed_ * decreaseAmount_;
return (intervalFullReward_ * (endTime_ - startTime_)) / interval_;
}
.
.
.
return firstPeriodReward_ + secondPeriodReward_ + thirdPeriodReward_;
}
The intervalsPassed_
is calculated as timePassedBefore_/interval
. Then the intervalsPassed_
is a part of calculation of intervalFullReward_
and is multiplied by decreaseAmount_
. In Solidity, the result of division is rounded down to the nearest integer. Therefore, the multiplication after division leads to incorrect result.
Impact
Let's consider the following input parameters for LinearDistributionIntervalDecrease::getPeriodReward
function:
initialAmount_ = 1000
decreaseAmount_ = 10
payoutStart_ = 100
interval_ = 45
startTime_ = 200
endTime_ = 300
With this parameters we will have:
timePassedBefore_
equals to 100. The if
statement will be true. intervalsPassed_
equals to 2
and intervalFullReward_
equals to 980
. The final result for getPeriodReward
will be 2177
.
Now, let's suppose we have multiplication before division or intervalFullReward_
will be:
uint256 intervalFullReward_ = initialAmount_ - timePassedBefore_ * decreaseAmount_ / interval_;
In that case intervalFullReward_
will be equal to 978
. And the returned value from getPeriodReward
will be 2173
. This result is lower than the previous one. So, the division before multiplication in this function leads to more period reward.
Tools Used
Manual Review
Recommendations
Rewrite the function in such a way that the multiplication is performed before division:
function getPeriodReward(
uint256 initialAmount_,
uint256 decreaseAmount_,
uint128 payoutStart_,
uint128 interval_,
uint128 startTime_,
uint128 endTime_
) external pure returns (uint256) {
if (interval_ == 0) {
return 0;
}
// 'startTime_' can't be less than 'payoutStart_'
if (startTime_ < payoutStart_) {
startTime_ = payoutStart_;
}
uint128 maxEndTime_ = _calculateMaxEndTime(payoutStart_, interval_, initialAmount_, decreaseAmount_);
if (endTime_ > maxEndTime_) {
endTime_ = maxEndTime_;
}
// Return 0 when calculation 'startTime_' is bigger then 'endTime_'...
if (startTime_ >= endTime_) {
return 0;
}
// Calculate interval that less then 'interval_' range
uint256 timePassedBefore_ = startTime_ - payoutStart_;
if ((timePassedBefore_ / interval_) == ((endTime_ - payoutStart_) / interval_)) {
+ uint256 intervalFullReward_ = initialAmount_ - timePassedBefore_ * decreaseAmount_ / interval_;
- uint256 intervalsPassed_ = timePassedBefore_ / interval_;
- uint256 intervalFullReward_ = initialAmount_ - intervalsPassed_ * decreaseAmount_;
return (intervalFullReward_ * (endTime_ - startTime_)) / interval_;
}
.
.
.
return firstPeriodReward_ + secondPeriodReward_ + thirdPeriodReward_;
}