DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Invalid

Unbounded Swap Deadline in `StrategyArb` and `StrategyOp` Allows Execution of Stale Transactions

Summary

The StrategyArb contract implements functionality to swap underlying WETH tokens for alETH (synthetic ETH) using the Ramses Router. However, in the _swapUnderlyingToAsset() function, the swap deadline is set to block.timestamp, which effectively removes the deadline protection against stale transactions.

This is particularly concerning in Layer 2 networks where network congestion can cause significant transaction delays. When a swap transaction gets delayed due to congestion, it can still be executed much later under potentially unfavorable market conditions, as the deadline check will always pass (the block's timestamp will always be >= itself).

The impact is most severe in volatile market conditions where delayed swaps could execute at significantly worse prices than intended. While the function does implement slippage protection via the minOut parameter, this alone is insufficient as it doesn't protect against the transaction being held and executed at a later time when the temporary price impact of other trades has subsided.

Proof of Concept

  1. User calls claimAndSwap() with parameters for a favorable swap

  2. Network becomes congested, transaction stays pending

  3. Market conditions change unfavorably

  4. Much later (could be hours), the transaction finally gets included

  5. Because block.timestamp was used as deadline, the swap executes despite being stale

  6. The strategy receives worse swap terms than intended, only bounded by minOut

Recommended mitigation steps

Add a deadline parameter to the claimAndSwap() and _swapUnderlyingToAsset() functions to allow users to specify their maximum acceptable delay. This deadline should be passed through to the router call:

- function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
+ function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path, uint256 _deadline) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
- _swapUnderlyingToAsset(_amountClaim, _minOut, _path);
+ _swapUnderlyingToAsset(_amountClaim, _minOut, _path, _deadline);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
- function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IRamsesRouter.route[] calldata _path) internal {
+ function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IRamsesRouter.route[] calldata _path, uint256 _deadline) internal {
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
- IRamsesRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
+ IRamsesRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), _deadline);
}
Updates

Appeal created

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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