Function withdrawToVault() in BaseStrategy will withdraw all funds from strategy to vault, it uses an internal function _withdrawAll() in MyStrategy. In this function, there is a check that no locked balance is still in AuraLocker.
An attacker can keep deposit small amount (1 wei) for MyStrategy in AuraLocker by calling lock(address(MyStrategy), 1) in AuraLocker to keep locked balance of MyStrategy never be 0, making the function withdrawToVault() being in denial of service.
Proof of Concept
Attacker keep calling lock(address(MyStrategy), 1) in AuraLocker every lockDuration.
balanceOfPool() always bigger than zero making the check in line 185 revert.
Vault calls withdrawToVault() will always revert
Recommended Mitigation Steps
Add manualProcessExpiredLocks in function _withdrawAll() and remove the check if no locked balance.
GalloDaSballo
commented
2 years ago
Lines of code
https://github.com/Badger-Finance/vested-aura/blob/b6abb069431518962e1e0b3e516daa46ae3bdd9b/contracts/MyStrategy.sol#L185
Vulnerability details
Impact
Function
withdrawToVault()in BaseStrategy will withdraw all funds from strategy to vault, it uses an internal function_withdrawAll()in MyStrategy. In this function, there is a check that no locked balance is still in AuraLocker.An attacker can keep deposit small amount (1 wei) for MyStrategy in AuraLocker by calling
lock(address(MyStrategy), 1)in AuraLocker to keep locked balance of MyStrategy never be 0, making the functionwithdrawToVault()being in denial of service.Proof of Concept
lock(address(MyStrategy), 1)in AuraLocker everylockDuration.withdrawToVault()will always revertRecommended Mitigation Steps
Add
manualProcessExpiredLocksin function_withdrawAll()and remove the check if no locked balance.