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

`sellProfits` function of `Fees` contract accepts any slippage

rotcivegaf

Summary

The sellProfits function accepts any slippage

Vulnerability Details

The sellProfits function have the amountOutMinimum and sqrtPriceLimitX96 in 0, also can be called by anyone, this gives the possibility of a MEV attack or sandwich attack

From Uniswap Docs:

amountOutMinimum: we are setting to zero, but this is a significant risk in production. For a real deployment, this value should be calculated using our SDK or an onchain price oracle - this helps protect against getting an unusually bad price for a trade due to a front running sandwich or another type of price manipulation

sqrtPriceLimitX96: We set this to zero - which makes this parameter inactive. In production, this value can be used to set the limit for the price the swap will push the pool to, which can help protect against price impact or for setting up logic in a variety of price-relevant mechanisms.

Another issue is the pool fee is always 3000(0.3%), which means that the pool tier is always the same, it may be the case that the liquidity of the pool of 3000(0.3%) is changed to the other, for example 100(0.01%) or 500(0.05%), giving a greater possibility of MEV attacks since it has low liquidity and also giving worse rates to token swaps

Impact

Loose the profits received by the Fees contract

Recommendations

Add the fee, amountOutMinimum and sqrtPriceLimitX96 parameters to the sellProfits function and calculate these off-chain

Also protect this function with as example onlyOwner

@@ -4,11 +4,13 @@ pragma solidity ^0.8.19;
import "./utils/Errors.sol";
import "./utils/Structs.sol";
+import {Ownable} from "./utils/Ownable.sol";
+
import {IERC20} from "./interfaces/IERC20.sol";
import {ISwapRouter} from "./interfaces/ISwapRouter.sol";
-contract Fees {
+contract Fees is Ownable {
address public immutable WETH;
address public immutable staking;
@@ -16,14 +18,19 @@ contract Fees {
ISwapRouter public constant swapRouter =
ISwapRouter(0xE592427A0AEce92De3Edee1F18E0157C05861564);
- constructor(address _weth, address _staking) {
+ constructor(address _weth, address _staking) Ownable(msg.sender) {
WETH = _weth;
staking = _staking;
}
/// @notice swap loan tokens for collateral tokens from liquidations
/// @param _profits the token to swap for WETH
- function sellProfits(address _profits) public {
+ function sellProfits(
+ address _profits,
+ uint24 _fee,
+ uint256 _amountOutMinimum,
+ uint160 _sqrtPriceLimitX96
+ ) public onlyOwner {
require(_profits != WETH, "not allowed");
uint256 amount = IERC20(_profits).balanceOf(address(this));
@@ -31,12 +38,12 @@ contract Fees {
.ExactInputSingleParams({
tokenIn: _profits,
tokenOut: WETH,
- fee: 3000,
+ fee: _fee,
recipient: address(this),
deadline: block.timestamp,
amountIn: amount,
- amountOutMinimum: 0,
- sqrtPriceLimitX96: 0
+ amountOutMinimum: _amountOutMinimum,
+ sqrtPriceLimitX96: _sqrtPriceLimitX96
});

Support

FAQs

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