Omniscia Keyko Audit

ImpactLabsVestingImplementation Manual Review Findings

ImpactLabsVestingImplementation Manual Review Findings

ILV-01M: Improper Last Claim Update

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:

contracts/token/ImpactLabsVestingImplementation.sol
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 covered
117 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 advance
121 // it decrease advancePayment value
122 // it doesn't transfer PACTs to ImpactLabs
123 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 advance
127 // it decrease advancePayment to 0
128 // it transfer the difference to ImpactLabs
129 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

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:

contracts/token/ImpactLabsVestingImplementation.sol
139/**
140 * @notice Transfers an amount of an ERC20 from this contract to an address
141 *
142 * @param _token address of the ERC20 token
143 * @param _to address of the receiver
144 * @param _amount amount of the transaction
145 */
146function transfer(
147 IERC20 _token,
148 address _to,
149 uint256 _amount
150) 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.