Incorrect calculation in terminate function
Description
In the function terminate in the VestingAllocation contract, the tokensToRecover is used to calculate the remaining tokens, excluding the tokens vested and withdrawn.
However, the getVestedTokenAmount() function returns the total vested tokens, so adding tokensWithdrawn is unnecessary. It allows the controller to recover more tokens than those remaining, causing the transaction to revert due to integer underflow.
function terminate() external override onlyController nonReentrant {
if(terminated) revert MetaVesT_AlreadyTerminated();
uint256 tokensToRecover = 0;
uint256 milestonesAllocation = 0;
for (uint256 i; i < milestones.length; ++i) {
milestonesAllocation += milestones[i].milestoneAward;
}
tokensToRecover = allocation.tokenStreamTotal + milestonesAllocation - getVestedTokenAmount() + tokensWithdrawn;
terminationTime = block.timestamp;
safeTransfer(allocation.tokenContract, getAuthority(), tokensToRecover);
terminated = true;
emit MetaVesT_Terminated(grantee, tokensToRecover);
}Similarly, in the function terminate in the TokenOptionAllocation contract, the tokensToRecover is used to calculate the remaining tokens, excluding the tokens exercisable and exercised. For the same reason, adding tokensWithdrawn is unnecessary.
function terminate() external override onlyController nonReentrant {
if(terminated) revert MetaVesT_AlreadyTerminated();
uint256 milestonesAllocation = 0;
for (uint256 i; i < milestones.length; ++i) {
milestonesAllocation += milestones[i].milestoneAward;
}
uint256 tokensToRecover = allocation.tokenStreamTotal + milestonesAllocation - getAmountExercisable() - tokensExercised + tokensWithdrawn;
terminationTime = block.timestamp;
shortStopTime = block.timestamp + shortStopDuration;
safeTransfer(allocation.tokenContract, getAuthority(), tokensToRecover);
terminated = true;
emit MetaVesT_Terminated(grantee, tokensToRecover);
}Impact
The controller cannot terminate the allocation contract and recover the remaining tokens.
Recommendations
Remove the tokensWithdrawn from the calculation of the remaining tokens in the terminate function.
Remediation
This issue has been acknowledged by MetaLeX Labs, Inc, and a fix was implemented in commit 033ab663↗.