Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/arbitrage/ArbitrageSearch.sol#L101

Vulnerability details

LoC

https://github.com/code-423n4/2024-01-salty/blob/main/src/arbitrage/ArbitrageSearch.sol#L101

Vulnerability Detail

The ArbitrageSearch contract's calculations for arbitrage opportunities are based on current market prices without considering potential price movements during transaction execution. Given the nature of blockchain transactions and the possibility of block delays, the actual execution price can differ from the calculated one.

Code Snippet

function _bisectionSearch( uint256 swapAmountInValueInETH, uint256 reservesA0, uint256 reservesA1, uint256 reservesB0, uint256 reservesB1, uint256 reservesC0, uint256 reservesC1 ) internal pure returns (uint256 bestArbAmountIn)
        {
        // This code can safely be made unchecked as the functionality for the found bestArbAmountIn is duplicated exactly in Pools._arbitrage with overflow checks kept in place.
        // If any overflows occur as a result of the calculations here they will happen in the Pools._arbitrage code.
        unchecked
            {
            if ( reservesA0 <= PoolUtils.DUST || reservesA1 <= PoolUtils.DUST || reservesB0 <= PoolUtils.DUST || reservesB1 <= PoolUtils.DUST || reservesC0 <= PoolUtils.DUST || reservesC1 <= PoolUtils.DUST )
                return 0;

            // Search bestArbAmountIn in a range from 1/128th to 125% of swapAmountInValueInETH.
            uint256 leftPoint = swapAmountInValueInETH >> 7;
            uint256 rightPoint = swapAmountInValueInETH + (swapAmountInValueInETH >> 2); // 100% + 25% of swapAmountInValueInETH

            // Cost is about 492 gas per loop iteration
            for( uint256 i = 0; i < 8; i++ )
                {
                uint256 midpoint = (leftPoint + rightPoint) >> 1;

                // Right of midpoint is more profitable?
                if ( _rightMoreProfitable( midpoint, reservesA0, reservesA1, reservesB0, reservesB1, reservesC0, reservesC1 ) )
                    leftPoint = midpoint;
                else
                    rightPoint = midpoint;
                }

            bestArbAmountIn = (leftPoint + rightPoint) >> 1;

            // Make sure bestArbAmountIn is actually profitable (taking into account precision errors)
            uint256 amountOut = (reservesA1 * bestArbAmountIn) / (reservesA0 + bestArbAmountIn);
            amountOut = (reservesB1 * amountOut) / (reservesB0 + amountOut);
            amountOut = (reservesC1 * amountOut) / (reservesC0 + amountOut);

            // @audit Implement slippage consideration here
    // Slippage should be considered in the final profitability check.
    // The actual amountOut should be compared against a slippage-adjusted expected output.
    // This ensures the arbitrage remains profitable even after accounting for potential price movements.

            if ( ( int256(amountOut) - int256(bestArbAmountIn) ) < int256(PoolUtils.DUST) )
                return 0;
            }
        }
    }

The _bisectionSearch function currently checks if the output (amountOut) minus the input (bestArbAmountIn) is greater than a dust threshold. However, this does not account for the potential slippage that could occur when the arbitrage trade is actually executed on the blockchain.

Before the final profitability check, incorporate a slippage model to adjust the expected output (amountOut). This adjustment should reflect the maximum allowable price movement to still deem the trade profitable.

Impact

In a volatile market, the price of tokens can change significantly between the time an arbitrage opportunity is identified and when the transaction is executed. This can lead to trades being executed at prices that are no longer profitable.

In extreme cases, the absence of slippage control could turn a theoretically profitable arbitrage into a loss-making trade. This occurs when the execution price slips beyond the point where the arbitrage trade yields profit.

Without slippage management, the capital allocated for arbitrage might not be used optimally, as trades could close at less than optimal prices, reducing overall profitability.

Exploitation Scenario

Other participants (especially bots) can observe an impending arbitrage transaction and execute trades that affect the prices of the involved assets, leading to slippage.
Rapid price changes in one or more assets involved in the arbitrage can occur naturally due to market volatility, leading to discrepancies between expected and actual trade outcomes.

The ArbitrageSearchcontract's approach to determining the arbitrage path and optimizing trade amounts does not dynamically account for these potential price shifts. This omission is particularly critical because DeFi markets are known for their high volatility and relatively thin liquidity compared to traditional markets.

Recommendations

Introducing parameters that define acceptable slippage limits, which can be dynamically adjusted based on market conditions or specific asset pairs involved.
Implementing a trade execution strategy that incorporates real-time price feeds and adjusts orders to reflect current market conditions, potentially using oracles or price prediction algorithms.
Re-evaluate the profitability of an arbitrage opportunity immediately before execution, considering the latest market data and slippage parameters.

Assessed type

Invalid Validation

code-423n4 / 2024-01-salty-findings

Inadequate Slippage Management in Arbitrage Calculations #678