VotingEscrow Manual Review Findings

VEW-01M: Inexistent Prevention of Approvals

Type	Severity	Location
Standard Conformity		VotingEscrow.sol:L271-L276, L278-L283

Description:

The VotingEscrow contract will permit IERC20::approve operations to be performed on it even though no transfers are ultimately executable.

Impact:

It is presently possible to approve VotingEscrow tokens even though the approval cannot be presently capitalized on.

Example:

Copy
271/**
272 * @notice Transfers of voting balances are not allowed.  This function will revert.
273 */
274function transfer(address, uint256) public pure override returns (bool) {
275    revert VotingEscrowTransferNotSupported();
276}
277
278/**
279 * @notice Transfers of voting balances are not allowed.  This function will revert.
280 */
281function transferFrom(address, address, uint256) public pure override returns (bool) {
282    revert VotingEscrowTransferNotSupported();
283}

Recommendation:

We advise approvals to be prevented as well.

Alternatively, we advise approvals to be treated as extension approvals for all lockupId values instead of permitting each one.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

Approvals are properly disallowed in the latest implementation by overriding the VotingEscrow::approve function, addressing this exhibit.

VEW-02M: Inexistent Validation of Multitude

Type	Severity	Location
Input Sanitization		VotingEscrow.sol:L177

Description:

The VotingEscrow::merge function does not ensure that the input lockupIds are at least 2, permitting merge operations to be performed for a single ID.

Impact:

The only security ramification that may result from the described misbehaviour would be from integrators of the VotingEscrow::merge function, rendering this exhibit to be informational in nature.

Example:

Copy
176/// @inheritdoc IMaverickV2VotingEscrowBase
177function merge(uint256[] memory lockupIds) public returns (Lockup memory newLockup) {
178    uint128 cumulativeAmount;
179    uint256 maxEnd;
180
181    Lockup memory oldLockup;
182    for (uint256 k; k < lockupIds.length; k++) {
183        oldLockup = _unstake(msg.sender, lockupIds[k]);
184        cumulativeAmount += oldLockup.amount;
185        maxEnd = Math.max(maxEnd, oldLockup.end);
186    }
187
188    // stake new lockup; checks to ensure new duration is at least min duration.
189    newLockup = _stake(cumulativeAmount, maxEnd - block.timestamp, msg.sender, lockupIds[0]);
190}

Recommendation:

We advise the VotingEscrow::merge function to ensure that the lockupIds.length is greater-than-or-equal-to (>=) the value of 2, ensuring all invocations of VotingEscrow::merge are properly performed.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The Maverick Protocol team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase