code-423n4 / 2022-03-paladin-findings

0 stars 0 forks source link

wrong calculation #25

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L1156

Vulnerability details

Impact

in _lock if the user set the duration to MIN_LOCK_DURATION it will fail because of durationRatio calculation it will try to divide zero /(MAX_LOCK_DURATION - MIN_LOCK_DURATION); .

Proof of Concept

https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L1156

Tools Used

Manual

Recommended Mitigation Steps

Kogaroshi commented 2 years ago

This is not an issue, since it's the desired behavior for the ratio This value should go from 0 to UNIT :

0 when at minimum Lock duration
UNIT when at maximum Lock duration

As it is then used on the next line to calculate the user BonusRatio based on the duration. a durationRatio of 0 (so for min lock), will simply give the minBonusRatio, as desired

Extra tests were written to prove the desired bonusRatio were applied: https://github.com/PaladinFinance/Paladin-Tokenomics/commit/7ae12484dc2dff8a75bc0342da13a6af3c2f20cf

0xean commented 2 years ago

   uint256 public constant MIN_LOCK_DURATION = 7889400; // 3 months
    /** @notice Maximum duration of a Lock  */
    uint256 public constant MAX_LOCK_DURATION = 63115200; // 2 years

Based on the above MAX - MIN can never be zero. Closing as invalid.