MaverickV2Reward Manual Review Findings

MVR-01M: Inexistent Conformity to Checks-Effects-Interactions Pattern

Type	Severity	Location
Standard Conformity		MaverickV2Reward.sol:L262, L264, L266

Description:

Although the present integration is internal and secure, the referenced statements do not adhere to the CEI pattern and will perform state changes after an external call.

Impact:

In the context of the Maverick Protocol, the present statements are secure. In any case, we still advise the CEI pattern to be adhered to as a best practice.

Example:

Copy
241/// @inheritdoc IMaverickV2Reward
242function pushUnboostedToVe(
243    uint8 rewardTokenIndex
244) public returns (uint128 amount, uint48 timepoint, uint256 batchIndex) {
245    IMaverickV2VotingEscrow ve = veTokenByIndex(rewardTokenIndex);
246    IERC20 token = rewardTokenByIndex(rewardTokenIndex);
247    RewardData storage data = rewardData[rewardTokenIndex];
248    amount = data.unboostedAmount;
249    if (amount == 0) revert RewardZeroAmount();
250    if (block.timestamp <= data.lastUnboostedPushTimestamp + UNBOOSTED_MIN_TIME_GAP) {
251        // revert if not enough time has passed; will not revert if this is
252        // the first call and last timestamp is zero.
253        revert RewardUnboostedTimePeriodNotMet(
254            block.timestamp,
255            data.lastUnboostedPushTimestamp + UNBOOSTED_MIN_TIME_GAP
256        );
257    }
258
259    timepoint = Time.timestamp();
260
261    token.forceApprove(address(ve), amount);
262    batchIndex = ve.createIncentiveBatch(amount, timepoint, ve.MAX_STAKE_DURATION().toUint128(), token);
263
264    data.unboostedAmount = 0;
265
266    data.lastUnboostedPushTimestamp = block.timestamp;
267}

Recommendation:

We advise the data related state changes to be performed prior to the MaverickV2VotingEscrow::createIncentiveBatch function call, ensuring that the system cannot be re-entered in a corrupt state during the reentrant MaverickV2Reward::pushUnboostedToVe function.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The statements have been re-ordered as advised, ensuring that the unboostedAmount and lastUnboostedPushTimestamp members of the RewardData data structure have been updated prior to the external interaction.

As such, we consider the code to be in full compliance with the CEI pattern and the exhibit to be fully alleviated.

MVR-02M: Insecure Stake Extension Authorization

Type	Severity	Location
Logical Fault		MaverickV2Reward.sol:L226-L235, L512

Description:

The current authorization system of the VotingEscrow::extendForAccount function is based on a caller validation model whereby the owner of a particular ve entry authorizes the caller to extend it. As the caller in the case of extensions by the MaverickV2Reward will be the MaverickV2Reward contract itself, any user can extend the ve entry of another user who has approved the MaverickV2Reward contract.

This is an insecure approach, as a malicious user can simply extend the duration of a victim's lock infinitely with a miniscule reward.

Impact:

It is presently possible to arbitrarily increase the lock duration of any user's VotingEscrow provided that they have approved the MaverickV2Reward contract due to expecting the MaverickV2Reward::getRewardForExistingVeLockup function to be used correctly.

Example:

Copy
226/// @inheritdoc IMaverickV2Reward
227function getRewardForExistingVeLockup(
228    uint256 tokenId,
229    address recipient,
230    uint8 rewardTokenIndex,
231    uint256 stakeDuration,
232    uint256 lockupId
233) external onlyTokenIdAuthorizedUser(tokenId) returns (RewardOutput memory) {
234    return _getReward(tokenId, recipient, rewardTokenIndex, stakeDuration, lockupId);
235}

Recommendation:

We advise the MaverickV2Reward::getRewardForExistingVeLockup function to be revised, either by being omitted entirely or by introducing an additional approval layer at the MaverickV2Reward level.

Alleviation (07ad29f773f16bdfbae3d97d3a7c2f9d64866093):

The Maverick Protocol team evaluated this exhibit, and opted to remove the MaverickV2Reward::getRewardForExistingVeLockup function entirely thereby alleviating this exhibit by omission.