Omniscia Keyko Audit
ImpactLabsVestingImplementation Manual Review Findings
ImpactLabsVestingImplementation Manual Review Findings
ILV-01M: Improper Last Claim Update
Type | Severity | Location |
---|---|---|
Logical Fault | Medium | ImpactLabsVestingImplementation.sol:L108, L136 |
Description:
The lastClaimedRewardPeriod
update within claim
is incorrect as it utilizes the total count of reward periods which will not represent the end of the while
loop iteration in case the condition _endBlock >= block.number
is satisfied a lot of reward periods before the last one.
Example:
95function claim() external override whenNotPaused {96 uint256 _index = lastClaimedRewardPeriod + 1;97 uint256 _rewardPeriodCount = donationMiner.rewardPeriodCount();98
99 uint256 _rewardPerBlock;100 uint256 _startBlock;101 uint256 _endBlock;102 uint256 _claimAmount;103
104 while (_index <= _rewardPeriodCount) {105 (_rewardPerBlock, , _startBlock, _endBlock, ) = donationMiner.rewardPeriods(_index);106
107 if (_endBlock >= block.number) {108 _rewardPeriodCount--;109 break;110 }111
112 _claimAmount += ((_endBlock - _startBlock + 1) * _rewardPerBlock * 3) / 4;113 _index++;114 }115
116 // if advancePayment is zero it means that all the entire amount payed in advance has been covered117 if (advancePayment == 0) {118 transferToImpactLabs(_claimAmount);119 } else if (advancePayment >= _claimAmount) {120 // if the claim amount is lesser than the amount of PACTs that is still given in advance121 // it decrease advancePayment value122 // it doesn't transfer PACTs to ImpactLabs123 advancePayment -= _claimAmount;124 emit AdvancePaymentDecreased(_claimAmount, advancePayment);125 } else {126 // if the claim amount is greater than the amount of PACTs that is still given in advance127 // it decrease advancePayment to 0128 // it transfer the difference to ImpactLabs129 uint256 toTransfer = _claimAmount - advancePayment;130 advancePayment = 0;131
132 emit AdvancePaymentDecreased(_claimAmount - toTransfer, 0);133 transferToImpactLabs(toTransfer);134 }135
136 lastClaimedRewardPeriod = _rewardPeriodCount;137}
Recommendation:
We advise the linked statements to use the _index
variable instead to properly account for unclaimed reward periods.
Alleviation:
The claim
function iterator has been adjusted to a dedicated variable that is properly maintained to the value of the _index
under all cases, thereby alleviating this exhibit.
ILV-02M: Potential Bypass of Vesting
Type | Severity | Location |
---|---|---|
Logical Fault | Minor | ImpactLabsVestingImplementation.sol:L146-L154 |
Description:
The transfer
function exposed by the contract permits the owner
to transfer an arbitrary token and an arbitrary amount to any recipient they wish so.
Example:
139/**140 * @notice Transfers an amount of an ERC20 from this contract to an address141 *142 * @param _token address of the ERC20 token143 * @param _to address of the receiver144 * @param _amount amount of the transaction145 */146function transfer(147 IERC20 _token,148 address _to,149 uint256 _amount150) external override onlyOwner nonReentrant {151 _token.safeTransfer(_to, _amount);152
153 emit TransferERC20(address(_token), _to, _amount);154}
Recommendation:
We advise this trait of the system to be re-evaluated and potentially prevented for the PACT
token as otherwise the vesting system can be bypassed.
Alleviation:
The Keyko team has stated that ownership of the contract is meant to be transferred to a Timelock address representing governance and as such, the code performs as expected thereby nullifying this exhibit.