Zellic - Audit Reports

Category: Business Logic

Reward tokens corresponding to removed milestones are locked

Critical Severity

High Impact

Medium Likelihood

Description

In the addMetavestMilestone and createMetavest functions, tokens are transferred to _grant as a milestone award. However, the removeMetavestMilestone function does not return the tokens associated with the removed milestone.

Impact

The grantee cannot get awards in removed milestones. The authority is also unable to recover these tokens because the calculation of tokensToRecover uses the value in storage instead of the token balance.

The following proof-of-concept script demonstrates that removed milestones' awards are left in the contract after the authority and grantee complete the withdrawal.

function testAuditRemainingBalanceFromRemoveMilestone() public {
    address vestingAllocation = createDummyVestingAllocationSlowUnlock();
    uint256 snapshot = token.balanceOf(authority);

    VestingAllocation(vestingAllocation).confirmMilestone(0);
    
    // add a new milestone and remove it
    BaseAllocation.Milestone[] memory milestones = new BaseAllocation.Milestone[](1);
    milestones[0] = BaseAllocation.Milestone({
        milestoneAward: 1337,
        unlockOnCompletion: false,
        complete: true,
        conditionContracts: new address[](0)
    }); 
    token.approve(address(controller), 1337);
    controller.addMetavestMilestone(vestingAllocation, milestones[0]);

    bytes4 selector = bytes4(keccak256("removeMetavestMilestone(address,uint256)"));
    bytes memory msgData = abi.encodeWithSelector(selector, vestingAllocation, 1);
    controller.proposeMetavestAmendment(vestingAllocation, controller.removeMetavestMilestone.selector, msgData);
    vm.prank(grantee);
    controller.consentToMetavestAmendment(vestingAllocation, controller.removeMetavestMilestone.selector, true);
    controller.removeMetavestMilestone(vestingAllocation, 1);

    vm.warp(block.timestamp + 25 seconds);
    controller.terminateMetavestVesting(vestingAllocation);
    
    // withdraw all vested tokens
    vm.startPrank(grantee);
    vm.warp(block.timestamp + 25 seconds);
    VestingAllocation(vestingAllocation).withdraw(
      VestingAllocation(vestingAllocation)
      .getAmountWithdrawable()
    );
    vm.stopPrank();

    assertNotEq(token.balanceOf(vestingAllocation), 0);
    console.log('remaining balance: ', token.balanceOf(vestingAllocation));
}

The following text is the result of the proof-of-concept script:

[PASS] testAuditRemainingBalanceFromRemoveMilestone() (gas: 2007632)
Logs:
  remaining balance:  1337

Recommendations

When removing a milestone, return the corresponding milestone award to the authority.

Remediation

This issue has been acknowledged by MetaLeX Labs, Inc, and a fix was implemented in commit dfe46863↗.