Category: Business Logic
Missing slippage limits allow front-running
Medium Severity
Medium Impact
Medium Likelihood
Description
During deposits and withdrawals, the staking manager interacts with the underlying vault for entry and exit. Those functions each have the parameter minAmount
, which sets slippage limits for the staker actions. However, the minAmount
is set to zero in all cases:
/**
* @dev Withdraw tokens from Definitive vault end-to-end (exit + withdraw)
*/
function withdrawFromDefinitive(
uint8 _index,
uint256 lpTokens
) private returns (uint256) {
IERC20 underlying = IERC20(underlyingTokenAddresses[_index]);
// 1. Exit from the strategy via LP Tokens
uint256 underlyingAmount = definitiveVault.exitOne(lpTokens, 0, _index);
// 2. Withdraw from the vault
definitiveVault.withdraw(underlyingAmount, address(underlying));
return underlyingAmount;
}
Impact
These slippage limits are essential for mitigating front-running. Consider the _processExitPoolTransfers
function in Balancer↗'s PoolBalances contract:
/**
* @dev Transfers `amountsOut` to `recipient`, checking that they are within their accepted limits, and pays
* accumulated protocol swap fees from the Pool.
*
* Returns the Pool's final balances, which are the current `balances` minus `amountsOut` and fees paid
* (`dueProtocolFeeAmounts`).
*/
function _processExitPoolTransfers(
address payable recipient,
PoolBalanceChange memory change,
bytes32[] memory balances,
uint256[] memory amountsOut,
uint256[] memory dueProtocolFeeAmounts
) private returns (bytes32[] memory finalBalances) {
finalBalances = new bytes32[](balances.length);
for (uint256 i = 0; i < change.assets.length; ++i) {
uint256 amountOut = amountsOut[i];
_require(amountOut >= change.limits[i], Errors.EXIT_BELOW_MIN);
...
}
...
}
If the minAmount
(which becomes an element of change.limits
) is set to zero, the slippage check does nothing. This leaves users vulnerable to front-running.
Recommendations
We recommend that the protocol provide users a way to specify minAmount
.
Remediation
This issue has been acknowledged by Rainmaker, and a fix was implemented in commit 2c613c09↗.