VaultRewards Manual Review Findings

VRS-01M: Inexistent Saturating Subtraction of Emergency Mechanism

Type	Severity	Location
Standard Conformity		VaultRewards.sol:L237

Description:

The VaultRewards::emergencyWithdraw mechanism should be as secure as possible yet performs a potentially reverting subtraction.

Impact:

The likelihood of this subtraction reverting is extremely low, however, the code should be future-proofed for a potential upgrade that might change how amounts are tracked.

Example:

Copy
231/// @notice Emergency withdraw without claiming rewards (forfeits pending AVO)
232function emergencyWithdraw() external nonReentrant {
233    UserInfo storage user = userInfo[msg.sender];
234    uint256 amount = user.amount;
235    if (amount == 0) revert ZeroAmount();
236
237    totalStaked -= amount;
238    user.amount = 0;
239    user.rewardDebt = 0;
240
241    stakeToken.safeTransfer(msg.sender, amount);
242    emit EmergencyWithdraw(msg.sender, amount);
243}

Recommendation:

We advise the code to try to subtract the full amount from the totalStaked, instead zeroing it out if there isn't a sufficient amount to subtract from.

Alleviation (a859cd2191d509cbc6d47508bdd44ec6d3cc9844):

The VaultRewards::emergencyWithdraw function was updated to perform safe operations throughout, ensuring it cannot fail due to an underflow arising from a corrupt system state.

VRS-02M: Inexistent Handling of Cap Breach

Type	Severity	Location
Logical Fault		VaultRewards.sol:L159

Description:

The VaultRewards::updatePool function does not properly handle the supply cap enforced by the AVOToken, causing the last reward mint operation to fail incorrectly.

Impact:

The last mint operation of the VaultRewards::updatePool will fail in its current implementation and will be unable to mint the last remaining rewards up to the token-configured cap.

Example:

Copy
146function updatePool() public {
147    if (block.timestamp <= lastRewardTime) return;
148
149    if (totalStaked == 0) {
150        lastRewardTime = block.timestamp;
151        return;
152    }
153
154    uint256 elapsed = block.timestamp - lastRewardTime;
155    uint256 rate = effectiveEmissionPerSecond();
156    uint256 reward = elapsed * rate;
157
158    // Mint AVO rewards
159    avoToken.mint(address(this), reward);
160    totalRewardsMinted += reward;
161
162    accRewardPerShare += (reward * 1e18) / totalStaked;
163    lastRewardTime = block.timestamp;
164}

Recommendation:

We advise the code mint whatever remainder up to the cap is available if the mint operation would fail, simultaneously ensuring the final mint occurs properly and preventing a DoS on normal pool operations from occurring.

Alleviation (a859cd2191d509cbc6d47508bdd44ec6d3cc9844):

The code was updated to cap the reward to the maximum supply mintable, alleviating this exhibit.

VRS-03M: Insecure Boost System

Type	Severity	Location
Logical Fault		VaultRewards.sol:L425-L428

Description:

The new boostScale system employed in VaultRewards cannot be compatible with updates of the boost multiplier despite what VaultRewards::setBoostScale implies as outstanding rewards are retroactively affected by the update.

Any account affected by a boost scale change will end up with incorrectly accounted rewards as the rewardDebt can decrease instead of increase.

Impact:

Any update of the boostScale will result in corrupted reward accounting.

Example:

Copy
423/// @notice Update the boost scale (AVO per avUSDC for max boost)
424/// @param _boostScale New boost scale value
425function setBoostScale(uint256 _boostScale) external onlyOwner {
426    boostScale = _boostScale;
427    emit BoostScaleUpdated(_boostScale);
428}

Recommendation:

We advise the update-able boostScale system to be removed, instead opting for a fixed-boost multiplier increase.

Alleviation (ca4268ecd382358c337d05fbb6e507c8789165ab):

The Avocado Fund team evaluated this exhibit and opted to remove the boost system entirely, alleviating this exhibit.