code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

`_zeroDurationPointBase` can potentially be exploited to get more scores #139

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

_zeroDurationPointBase can be set at deploy time so that locks with 0 duration can get scores.

However, if the value of _zeroDurationPointBase is being set high enough. It can potentially be exploited by repeatedly lock(), and unlock() with 0 duration to get scores.

This can get amplified with flashloans.

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L245-L247

function _getPoints(uint256 amount_, uint256 duration_) internal view returns (uint256 points_) {
    return amount_ * (duration_ + _zeroDurationPointBase);
}

Recommendation

Consider changing _zeroDurationPointBase to a constant of value 1.

deluca-mike commented 2 years ago

I thought about it some more and I have to agree that if we did allow a 0 duration, this entire score system is unenforceable due to flash loans. So, because of that, I'm going to make the score function simply amount_ * duration_, and remove _zeroDurationPointBase from the contract. If we wanted "mimial" lock duration, we can simply do something small like 1 day, or even 1 second. And if we did allow 0 seconds, then it's only fair that "flash loaner" gets an NFT of 0 score.

deluca-mike commented 2 years ago

In the release candidate contract, _zeroDurationPointBase has been removed, and it is no longer possible to lock for 0 duration, since enabling such a duration via setLockPeriods is prevented.