Lines of code
<https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/AfEth.sol#L272&gt;

Vulnerability details

Summary

Some functions that are part of the Votium reward flow are left unprotected and can be accessed by anyone to spend resources held by the contract.

Impact

Rewards coming from the Votium protocol are claimed and compounded back in AfEth. This flow consists of two parts, both controlled and initiated by the rewarder role: first, rewards are claimed in Votium and Convex using claimRewards(), second, those rewards are swapped to ETH and deposited back in the protocol using applyRewards().

After rewards are swapped, the VotiumStrategy will call AfEth to manage the deposited rewards, which eventually calls back the VotiumStrategy. These interactions are represented in the previous diagram as steps 5 and 6.

However, both of the functions that implement these steps are publicly accessible and don’t have any validation over the amount of ETH sent. Let’s first see the case of AfEth::depositRewards():

272:     function depositRewards(uint256 _amount) public payable {
273:        IVotiumStrategy votiumStrategy = IVotiumStrategy(vEthAddress);
274:         uint256 feeAmount = (_amount * protocolFee) / 1e18;
275:         if (feeAmount > 0) {
276:             // solhint-disable-next-line
277:             (bool sent, ) = feeAddress.call{value: feeAmount}("");
278:             if (!sent) revert FailedToSend();
279:         }
280:         uint256 amount = _amount - feeAmount;
281:         uint256 safEthTvl = (ISafEth(SAF_ETH_ADDRESS).approxPrice(true) *
282:             safEthBalanceMinusPending()) / 1e18;
283:         uint256 votiumTvl = ((votiumStrategy.cvxPerVotium() *
284:             votiumStrategy.ethPerCvx(true)) *
285:             IERC20(vEthAddress).balanceOf(address(this))) / 1e36;
286:         uint256 totalTvl = (safEthTvl + votiumTvl);
287:         uint256 safEthRatio = (safEthTvl * 1e18) / totalTvl;
288:         if (safEthRatio < ratio) {
289:             ISafEth(SAF_ETH_ADDRESS).stake{value: amount}(0);
290:         } else {
291:             votiumStrategy.depositRewards{value: amount}(amount);
292:         }
293:     }

As we can see in the previous snippet of code, the function doesn’t have any access control and doesn’t check if the _amount parameter matches the amount of ETH being sent (msg.value). Anyone can call this function with any amount value without actually sending any ETH value.

The implementation of depositRewards() in VotiumStrategyCore has the same issue:

203:     function depositRewards(uint256 _amount) public payable {
204:         uint256 cvxAmount = buyCvx(_amount);
205:         IERC20(CVX_ADDRESS).approve(VLCVX_ADDRESS, cvxAmount);
206:         ILockedCvx(VLCVX_ADDRESS).lock(address(this), cvxAmount, 0);
207:         emit DepositReward(cvxPerVotium(), _amount, cvxAmount);
208:     }

Any ETH held in these two contracts can be arbitrarily spent by any unauthorized account. The caller cannot remove value from here, unless sandwiching the trade or benefitting via a third-party call, but can use these functions to grief and unauthorizedly spend any ETH present in these contracts.

Recommendation

If these functions are indeed intended to be publicly accessible, then add a validation to ensure that the amount argument matches the callvalue sent, i.e. require(_amount == msg.value).

On the other hand, if these should only be part of the reward flow initiated by the rewarder role, then validate that AfEth::depositRewards() is called from the Votium Strategy (vEthAddress), and validate that VotiumStrategy::depositRewards() is called either from AfEth (manager) or internally through applyRewards().

Assessed type

Access Control

---

The text was updated successfully, but these errors were encountered:

All reactions