`sellCVX(0)` reverts

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/6b4867491350f8327d0ac4f496f263642cf3c1be/contracts/strategies/votium/VotiumStrategyCore.sol#L261

Vulnerability details

Impact

A withdrawal cannot be finalised if requested at a time when AfEth had only safEth, and that owed share of safEth is permanently lost.

Proof of Concept

It is possible that AfEth holds at most dust amounts of vAfEth (if ratio = 100 %). The amounts of vAfEth and safEth to withdraw is set at the time of the request. So if a withdrawal request is placed at this time then AfEth.requestWithdraw() will call VotiumStrategy.requestWithdraw(0) AfEth.sol#L190-L193

uint256 votiumWithdrawAmount = (withdrawRatio * votiumBalance) / 1e18;
uint256 vEthWithdrawId = AbstractStrategy(vEthAddress).requestWithdraw(
    votiumWithdrawAmount
);

which naturally records a cvxOwed: 0.

When later attempting to withdraw, in VotiumStrategy.withdraw() VotiumStrategy.sol#L119-L122

uint256 cvxWithdrawAmount = withdrawIdToWithdrawRequestInfo[_withdrawId]
    .cvxOwed;

uint256 ethReceived = sellCvx(cvxWithdrawAmount);

sellCvx(0) is called.

The issue is that this attempts to VotiumStrategyCore.sol#L258-L263

ICrvEthPool(CVX_ETH_CRV_POOL_ADDRESS).exchange_underlying(
    1,
    0,
    0, //@audit dx
    0 // this is handled at the afEth level
);

which reverts as dx == 0.

Since a withdrawal request is permanent and cannot be amended the user will never be able to withdraw his safEth share.

Recommended Mitigation Steps

For gas considerations this has to be addressed by checks along the following pathway. In AfEth.requestWithdraw(), skip the call to VotiumStrategy.requestWithdraw() if votiumWithdrawAmount == 0. I don't think cvxPerVotium() can be less than 1e18, but if it can then one also has to check that cvxAmount is non-zero [in VotiumStrategy.requestWithdraw()]() and return without placing the request in VotiumStrategy. This would place an empty request which has to not revert when later called in VotiumStrategy.withdraw().

It is of course possible to let everything run and just amend sellCvx(0) to immediately return 0.

Assessed type

Invalid Validation

[c4-judge]

0xleastwood marked the issue as primary issue

[c4-judge]

0xleastwood marked the issue as duplicate of #36

[c4-judge]

0xleastwood marked the issue as satisfactory

[c4-judge]

0xleastwood changed the severity to 3 (High Risk)