Alchemix Transmuter
Alchemix
DeFiFoundrySolidity
16,653 OP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Title Using `block.timestamp` for deadline can lead to frontrunning.

tdey

Summary

The _swapUnderlyingToAsset function in StrategyArb.sol & _swapUnderlyingToAsset uses block.timestamp as deadline parameter for interaction with AMM, miner can hold the txs until it becomes favorable for them.

Vulnerability Details

The _swapUnderlyingToAsset function in StrategyArb.sol :

function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IRamsesRouter.route[] calldata _path) internal {
​
// TODO : we swap WETH to ALETH -> need to check that price is better than 1:1
​
// uint256 oraclePrice = 1e18 * 101 / 100;
​
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);
​
//@audit using block.timestamp as deadline
​
}

The The _swapUnderlyingToAsset function in StrategyOp.sol :

function _swapUnderlyingToAsset(
uint256 _amount,
uint256 minOut,
IVeloRouter.route[] calldata _path
​
) internal {
​
// TODO : we swap WETH to ALETH -> need to check that price is better than 1:1
​
// uint256 oraclePrice = 1e18 * 101 / 100;
​
require(minOut > _amount, "minOut too low");
​
uint256 underlyingBalance = underlying.balanceOf(address(this));
​
require(underlyingBalance >= _amount, "not enough underlying balance");
​
@>IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp//@audit using block.timestamp for swapping tokens);
​
}

Deadlines are a helpful tool to make sure txn cannot be saved for later. Here block.timestamp is passed to a pool, therefore it will be valid whenever the miner chooses to include the txn in a block because there is no deadline expiration check. And block.timestamp will be the current timestamp at that moment. It can be more profitable for a miner to hold the txn, until the txn gets maximum slippage.
Similar issues :

  1. https://github.com/sherlock-audit/2023-02-bond-judging/issues/60

  2. https://code4rena.com/reports/2022-11-paraspace#m-13-interactions-with-amms-do-not-use-deadlines-for-operations

Impact

Txn maybe held by a malevolent miner, which may be being done in order to free up capital to ensure that there are funds available to do operations to prevent a liquidation. I am keeping the severity medium as similar issues in the past have the same severity.

Tools used

Manual review

Recommended mitigation

Add deadline parameter to _swapUnderlyingToAsset function, and pass it along AMM calls.

Here is a good example of deadline check in UniswapV2, it uses a modifier for the deadline check and all function which require the check, the modifier is being used.
https://github.com/Uniswap/v2-periphery/blob/0335e8f7e1bd1e8d8329fd300aea2ef2f36dd19f/contracts/UniswapV2Router02.sol#L18-L21
https://github.com/Uniswap/v2-periphery/blob/0335e8f7e1bd1e8d8329fd300aea2ef2f36dd19f/contracts/UniswapV2Router02.sol#L61-L76

Updates

Appeal created

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

Support

FAQs

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