Contract fails to deliver promised returns, but doesn't lose value
Description
Brief/Intro
The _deallocate() function in MorphoYearnOGWETHStrategy.sol contains a logic error where both balance checks (wethBalanceBefore and wethBalanceAfter) are performed after the vault withdrawal operation, rather than before and after. This causes the calculated wethRedeemed value to always equal zero, completely breaking the loss detection mechanism and causing false event emissions.
Vulnerability Details
Since both wethBalanceBefore and wethBalanceAfter are measured at the same point in time (after the withdrawal), they will always contain the same value, making wethRedeemed = 0 in all cases.
function _deallocate(uint256 amount) internal override returns (uint256) {
emit StrategyDeallocationLoss("Strategy deallocation loss.", amount, wethRedeemed); } require(wethRedeemed + wethBalanceBefore >= amount, "Strategy balance is less than the amount needed"); require(TokenUtils.safeBalanceOf(address(weth), address(this)) >= amount, "Strategy balance is less than the amount needed"); TokenUtils.safeApprove(address(weth), msg.sender, amount); return amount; }
Impact Details
1.Broken Loss Detection: The contract cannot detect if the vault returns less WETH than requested due to slippage, fees, or vault issues. This is a critical safety mechanism that is completely non-functional.
2.Redundant Safety Check: The require(wethRedeemed + wethBalanceBefore >= amount) check on line 54-55 becomes meaningless because it essentially checks require(0 + currentBalance >= amount), which is then immediately followed by the redundant check
Reccomendation
uint256 wethBalanceBefore = TokenUtils.safeBalanceOf(address(weth), address(this)); // Check BEFORE vault.withdraw(amount, address(this), address(this)); // Then withdraw uint256 wethBalanceAfter = TokenUtils.safeBalanceOf(address(weth), address(this)); // Check AFTER
Proof of Concept
Proof of Concept
function test_ProveWethRedeemedIsAlwaysZero() public { // Setup: Give strategy 100 WETH in vault weth.mint(address(strategy), 100 ether); strategy.allocate(100 ether);