DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: low
Valid

Strategy can miss capturing funds from positive slippage due to no deadline check on swaps

Summary

Strategies have function claimAndSwap which is callable by keepers. Keeper provides arg _minOut which is used to control for slippage. However there's no way for keeper to control the transaction expiration. This is particulary problematic on a mainnet since keeper's transaction can stay in a mempool for a prolonged time during which market condtion can change. When transaction is finally executed, outdated value of _minOut is used and thus strategy can get much less of asset out of swap than what it should receive.

Vulnerability Details

Setting the deadline is very important part of executing swaps. Here's a simple scenario where not setting the (proper) deadline can hurt the user:

  • Alice wants to swap 1 WETH for USDC. Maket rate is 2500 USDC/WETH

  • Alice provides 1 WETH, sets minOut to 2475 USDC (1% acceptable slippage). But Alice provides no deadline (or uses block.timestamp as a non-effective deadline)

  • there is a gas price spike so Alice's transaction is temporary stuck in mempool

  • in the meantime market changes and now rate is 3000 USDC/WETH

  • gas price drops back to level where Alice's TX can be included again

  • Bob, the MEV bot operator, notices Alices TX and sandwhiches it to capture MEV. As a consequence, Alice receives ~2500 USDC instead of expected ~3000 USDC

In above example Alice lost ~500 USDC because outdated slippage protection (minOut) was used. If Alice properly provided deadline, this situation wouldn't be possible.

Same can happen to the StrategyMainnet when keeper calls claimAndSwap. Function will protect the swapper in following ways:

  • minOut amount of alETH will surely be higher than amount of WETH put in

  • actual amount of alETH received will be at least minAmount

But there is no expiration check, so exact scenario described before can happen. Ie. keeper tries to claim 100 WETH and swap it for 102 alETH according to current market rate. TX gets temporary stuck in mempool and market rate changes to 108 alETH for 100 WETH. TX gets executed but startegy still only received 102 alETH due to outdated slippage protection. Strategy and its depositors effectively lost 6 alETH

Impact

Loss of funds for strategy (and all its depositors) because swap between WETH->alETH is executed at a lower rate compared to the available maket rate.

Tools Used

Manual review

Recommendations

Add parameter deadline to function claimAndSwap. In mainnet startegy, check that block.timestamp is less then provided deadline. In arb and op strategies, pass on the received deadline to Ramses/Velo router.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago

Appeal created

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
goran Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

claimAndSwap should have slippage on Mainnet

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.