Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Improper `deadline` check allows user transactions to be maliciously executed at a later time

madalad

Summary

AMMs such as Uniswap provide their users with an option to limit the execution of their pending actions, such as swaps or adding and removing liquidity. The most common solution is to include a deadline timestamp as a parameter (for example see Uniswap V2 and Uniswap V3). If such an option is not present, users can unknowingly perform bad trades.

Vulnerability Details

Fees#sellProfits executes a swap using Uniswap V3, but sets the deadline parameter as block.timestamp. This means that at execution time, the deadline check implemented by Uniswap will always pass, effectively meaning there is no deadline.

File: src\Fees.sol
​
24: /// @notice swap loan tokens for collateral tokens from liquidations
25: /// @param _profits the token to swap for WETH
26: function sellProfits(address _profits) public {
27: require(_profits != WETH, "not allowed");
28: uint256 amount = IERC20(_profits).balanceOf(address(this));
29:
30: ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
31: .ExactInputSingleParams({
32: tokenIn: _profits,
33: tokenOut: WETH,
34: fee: 3000,
35: recipient: address(this),
36: deadline: block.timestamp, // @audit unsafe deadline
37: amountIn: amount,
38: amountOutMinimum: 0,
39: sqrtPriceLimitX96: 0
40: });
41:
42: amount = swapRouter.exactInputSingle(params);
43: IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this)));
44: }

Without setting a proper deadline, the transaction can be executed arbitrarily far into the future. It is not uncommon for transactions to sit idle in the mempool for a long time before being confirmed, especially during times when gas price is high.

The following scenario describes the issue:

  1. Alice wants to swap 100 tokens for 1 ETH and later sell the 1 ETH for 1000 DAI.

  2. The transaction is submitted to the mempool, however, Alice chose a transaction
    fee that is too low for miners to be interested in including her transaction in a
    block. The transaction stays pending in the mempool for extended periods, which
    could be hours, days, weeks, or even longer.

  3. When the average gas fee dropped far enough for Alice's transaction to become
    interesting again for miners to include it, her swap will be executed. In the
    meantime, the price of ETH could have drastically changed. She will still get 1
    ETH but the DAI value of that output might be significantly lower. She has
    unknowingly performed a bad trade due to the pending transaction she forgot about.

An even worse way this issue can be maliciously exploited is through MEV:

  1. The swap transaction is still pending in the mempool. Average fees are still
    too high for miners to be interested in it. The price of tokens has gone up
    significantly since the transaction was signed, meaning Alice would receive a
    lot more ETH when the swap is executed. But that also means that her maximum
    slippage value is outdated and would allow for significant slippage.

  2. A MEV bot detects the pending transaction. Since the outdated maximum
    slippage value now allows for high slippage, the bot sandwiches Alice, resulting
    in significant profit for the bot and significant loss for Alice.

Impact

Each time sellProfits is called, there is a risk that the transaction will not be executed for a long time and a large amount of value will be lost.

Tools Used

Manual review

Recommendations

Allow the caller of sellProfits to specify a deadline by adding a parameter:

- function sellProfits(address _profits) public {
+ function sellProfits(address _profits, uint256 _deadline) public {
require(_profits != WETH, "not allowed");
uint256 amount = IERC20(_profits).balanceOf(address(this));
​
ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
.ExactInputSingleParams({
tokenIn: _profits,
tokenOut: WETH,
fee: 3000,
recipient: address(this),
- deadline: block.timestamp,
+ deadline: _deadline,
amountIn: amount,
amountOutMinimum: 0,
sqrtPriceLimitX96: 0
});
​
amount = swapRouter.exactInputSingle(params);
IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this)));
}

Support

FAQs

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