Anyone can extend withdraw wait period by depositing zero collateral #69
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Handle
harleythedog
Vulnerability details
Impact
In MochiVault.sol, the deposit function allows anyone to deposit collateral into any position. A malicious user can call this function with amount = 0, which would reset the amount of time the owner has to wait before they can withdraw their collateral from their position. This is especially troublesome with longer delays, as a malicious user would only have to spend a little gas to lock out all other users from being able to withdraw from their positions, compromising the functionality of the contract altogether.
Proof of Concept
deposit function here: https://github.com/code-423n4/2021-10-mochi/blob/main/projects/mochi-core/contracts/vault/MochiVault.sol#:~:text=function-,deposit,-(uint256%20_id%2C%20uint256
Notice that calling this function with amount = 0 is not disallowed. This overwrites lastDeposit[_id], extending the wait period before a withdraw is allowed.
Tools Used
Manual inspection.
Recommended Mitigation Steps
I would recommend adding:
require(amount > 0, "zero")
at the start of the function, as depositing zero collateral does not seem to be a necessary use case to support.
It may also be worthwhile to consider only allowing the owner of a position to deposit collateral.
The text was updated successfully, but these errors were encountered: