Forced relock in VotiumStrategy withdrawal causes denial of service if Convex locking contract is shutdown #50
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
M-03
primary issue
Highest quality submission among a set of duplicates
selected for report
This submission will be included/highlighted in the audit report
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Lines of code
https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L135-L149
Vulnerability details
Summary
The VotiumStrategy withdrawal process involves relocking CVX tokens, which can potentially lead to a denial of service and loss of user funds if the underlying vlCVX contract is shutdown.
Impact
When withdrawals are executed in VotiumStrategy, the implementation of
withdraw()
will callrelock()
in order to relock any available excess (i.e. expired tokens minus the pending obligations) of CVX tokens to lock them again in the Convex protocol.https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategy.sol#L135-L149
This seems fine at first, but if we dig into the implementation of
lock()
we can see that the preconditions of this function requires the contract not to be shutdown:https://etherscan.io/address/0x72a19342e8F1838460eBFCCEf09F6585e32db86E#code#L1469
Which means that any call to
lock()
after the contract is shutdown will revert. This is particularly bad becauserelock()
is called as part of the withdraw process. If the vlCVX contract is shutdown, VotiumStrategy depositors won't be able to withdraw their position, causing a potential loss of funds.Note that this also will cause a DoS while depositing rewards, since
depositRewards()
in VotiumStrategy also callsILockedCvx::lock()
.Proof of Concept
requestWithdrawal()
withdraw()
when the withdrawal epoch is reached.relock()
, which will callILockedCvx::lock()
.lock()
will throw an error because the vault is already shutdown.Recommendation
Add a condition to check if the contract is shutdown to avoid the call to
lock()
and the potential denial of service.Assessed type
DoS
The text was updated successfully, but these errors were encountered: