search

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

0 stars 0 forks source link

QA Report #60

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Pin Solidity Version

Consider to pin Solidity version to latest 0.8.12

Divide before Multiply

HolyPaladinToken._updateDropPerSecond() (contracts/HolyPaladinToken.sol#714-742) performs a multiplication on the result of a division: -dropDecreasePerMonth = (startDropPerSecond - endDropPerSecond) / (dropDecreaseDuration / MONTH) (contracts/HolyPaladinToken.sol#729) -nbMonthEllapsed = (block.timestamp - lastDropUpdate) / MONTH (contracts/HolyPaladinToken.sol#730) -dropPerSecondDecrease = dropDecreasePerMonth nbMonthEllapsed (contracts/HolyPaladinToken.sol#732) HolyPaladinToken._getNewIndex(uint256) (contracts/HolyPaladinToken.sol#744-761) performs a multiplication on the result of a division: -baseDropPerSecond = (_currentDropPerSecond UNIT) / maxLockBonusRatio (contracts/HolyPaladinToken.sol#752) -accruedBaseAmount = ellapsedTime baseDropPerSecond (contracts/HolyPaladinToken.sol#755) HolyPaladinToken._getUserAccruedRewards(address,uint256) (contracts/HolyPaladinToken.sol#786-851) performs a multiplication on the result of a division: -lockingRewards = (userLockedBalance ((indexDiff vars.periodBonusRatio) / UNIT)) / UNIT (contracts/HolyPaladinToken.sol#842) HolyPaladinToken._lock(address,uint256,uint256,HolyPaladinToken.LockAction) (contracts/HolyPaladinToken.sol#1137-1233) performs a multiplication on the result of a division: -durationRatio = ((duration - MIN_LOCK_DURATION) UNIT) / (MAX_LOCK_DURATION - MIN_LOCK_DURATION) (contracts/HolyPaladinToken.sol#1156) -userLockBonusRatio = minLockBonusRatio + (((maxLockBonusRatio - minLockBonusRatio) durationRatio) / UNIT) (contracts/HolyPaladinToken.sol#1157) HolyPaladinToken._lock(address,uint256,uint256,HolyPaladinToken.LockAction) (contracts/HolyPaladinToken.sol#1137-1233) performs a multiplication on the result of a division: -durationRatio_scope_0 = ((duration - MIN_LOCK_DURATION) UNIT) / (MAX_LOCK_DURATION - MIN_LOCK_DURATION) (contracts/HolyPaladinToken.sol#1212) -userLockBonusRatio_scope_1 = minLockBonusRatio + (((maxLockBonusRatio - minLockBonusRatio) durationRatio_scope_0) / UNIT) (contracts/HolyPaladinToken.sol#1213) HolyPaladinToken._kick(address,address) (contracts/HolyPaladinToken.sol#1270-1315) performs a multiplication on the result of a division: -nbWeeksOverLockTime = (block.timestamp - userCurrentLockEnd) / WEEK (contracts/HolyPaladinToken.sol#1305) -penaltyPercent = nbWeeksOverLockTime kickRatioPerWeek (contracts/HolyPaladinToken.sol#1306) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#divide-before-multiply

Used block.timestamp instead of block.number

Same result but inconsistant with comment https://github.com/code-423n4/2022-03-paladin/blob/9c26ec8556298fb1dc3cf71f471aadad3a5c74a0/contracts/HolyPaladinToken.sol#L861

Missing event on critical parameter change

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

Lack input validation on _rewardsVault

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

Kogaroshi commented 2 years ago

QA & gas optimizations changes are done in the PR: https://github.com/PaladinFinance/Paladin-Tokenomics/pull/6 (some changes/tips were implemented, others are noted but won't be applied)