Vesting Manual Review Findings

VGN-01M: Inexistent Claim of Existing Funds

Type	Severity	Location
Logical Fault		Vesting.sol:L100-L104

Description:

The Vesting::transferRecipient function will not claim any pending funds before transferring, indicating potentially undesirable behaviour.

Impact:

The severity and validity of this exhibit will be based on the action that the Mantissa team takes to alleviate it.

Example:

Copy
100function transferRecipient(address _newRecipient) external onlyRecipient {
101    emit RecipientTransferred(recipient, _newRecipient);
102    require(_newRecipient != address(0), "recipient zero address");
103    recipient = _newRecipient;
104}

Recommendation:

We advise the Mantissa team to evaluate this trait and either automatically claim any pending vest or retain the current behaviour in the codebase.

Alleviation (418ee413ad8e26f7eea383764c19953ff31b2bf3):

The Mantissa Finance team evaluated this exhibit and stated that the Vesting::transferRecipient function is a fail-safe and that the onus of any issues arising from it is on the sender and recipient.

As such, we consider this exhibit acknowledged.

VGN-02M: Potentially Unsafe Transfer of Vest

Type	Severity	Location
Logical Fault		Vesting.sol:L56-L60, L100-L104

Description:

The Vesting::transferRecipient function is meant to be utilized to transfer ownership of a particular vest, however, the Vesting::revoke and Vesting::withdraw functions can be utilized to cancel the vest and acquire any funds within it.

Impact:

It is presently possible to advertise a Vesting for "sale" with the owner able to recoup any funds associated with it after the transfer of it concludes.

Example:

Copy
56function revoke() external onlyOwner {
57    require(canBeRevoked, "Cannot be revoked");
58    emit AmountRevoked(amountRemaining);
59    amountRemaining = 0;
60}
61
62function withdraw(address _token) external onlyOwner {
63    uint256 contractBalance = IERC20(_token).balanceOf(address(this));
64    require(contractBalance > 0, "Contract balance is already 0");
65    if (_token == mnt) {
66        require(contractBalance > amountRemaining, "Nothing to withdraw");
67        uint256 safeAmount = contractBalance - amountRemaining;
68        IERC20(_token).safeTransfer(msg.sender, safeAmount);
69    } else {
70        IERC20(_token).safeTransfer(msg.sender, contractBalance);
71    }
72}
73
74function getTotalAmountVested() public view returns (uint256) {
75    if (cliffTimestamp >= block.timestamp) {
76        return 0;
77    }
78    uint256 vestTime = block.timestamp - cliffTimestamp;
79    uint256 amountVested = amount * vestTime / vestDuration;
80    if (amountVested > amount) {
81        amountVested = amount;
82    }
83    return amountVested;
84}
85
86function getAmountClaimable() public view returns (uint256) {
87    uint256 totalVested = getTotalAmountVested();
88    uint256 amountPaid = amount - amountRemaining;
89    return totalVested - amountPaid;
90}
91
92function claim() external onlyRecipient {
93    uint256 claimable = getAmountClaimable();
94    require(claimable > 0, "Nothing to claim");
95    amountRemaining -= claimable;
96    IERC20(mnt).safeTransfer(recipient, claimable);
97    emit AmountClaimed(claimable, amountRemaining);
98}
99
100function transferRecipient(address _newRecipient) external onlyRecipient {
101    emit RecipientTransferred(recipient, _newRecipient);
102    require(_newRecipient != address(0), "recipient zero address");
103    recipient = _newRecipient;
104}

Recommendation:

We advise the code to disallow a transfer of the recipient if the Vesting can be revoked to prevent potential social engineering attacks, such as "selling" a Vesting that is revoked once the recipient acquires it.

Alleviation (418ee413ad8e26f7eea383764c19953ff31b2bf3):

The Mantissa Finance team stated that the responsible party for invoking the Vesting::revoke and Vesting::withdraw functions will be a multi-signature wallet owned by the Mantissa Finance team.

Additionally, they state that if a vesting is marked as revoke-able it implies that the Mantissa Finance team will be capable of updating it at any moment.

We would like to note that certain on-chain edge-case race conditions can still materialize (i.e. the multi-signature wallet issues a revocation but the user "sells" their vest before the revocation goes through) and as such, we consider this exhibit valid and acknowledged.