Description

The function removeMilestone uses the keyword delete to remove milestones. Since milestones is an array, delete milestones[_milestoneIndex] only deletes the milestone at _milestoneIndex and leaves the length of the array untouched.

This will cause checks using milestones.length to be inaccurate because the number of existing milestones may be less than the array length.

function removeMilestone(uint256 _milestoneIndex) external onlyController {
    if(terminated) revert MetaVesT_AlreadyTerminated();
    if (_milestoneIndex >= milestones.length) revert MetaVesT_ZeroAmount();
!   delete milestones[_milestoneIndex];
    emit MetaVesT_MilestoneRemoved(grantee, _milestoneIndex);
}

Impact

Conditional control statements that use the index and milestones.length to determine whether a milestone exists may mistakenly assume that the milestone exists even though it has been removed.

Recommendations

Consider using the member function pop to remove milestones from the array. The following code is an example:

milestones[_milestoneIndex] = milestones[milestones.length - 1]
milestones.pop()

Also note that since the index of milestones may change, the logic of other parts needs to be adjusted accordingly.

Remediation

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