Arbitrage may not be profitable

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L290

Vulnerability details

The Salty protocol incorporates an arbitrage operation each time a user initiates a swap. Currently, users cover the costs associated with this arbitrage operation, and the resulting profits are retained in _userDeposits[address(dao)][weth] (as indicated by the second arrow below). to be later divided between the DAO, SALT stakers and liquidity providers in DAO.performUpkeep:

function _arbitrage(IERC20 arbToken2, IERC20 arbToken3, uint256 arbitrageAmountIn ) internal returns (uint256 arbitrageProfit)
        {
        uint256 amountOut = _adjustReservesForSwap( weth, arbToken2, arbitrageAmountIn );
        amountOut = _adjustReservesForSwap( arbToken2, arbToken3, amountOut );
        amountOut = _adjustReservesForSwap( arbToken3, weth, amountOut );

        // Will revert if amountOut < arbitrageAmountIn
        arbitrageProfit = amountOut - arbitrageAmountIn;   <--------

        // Deposit the arbitrage profit for the DAO - later to be divided between the DAO, SALT stakers and liquidity providers in DAO.performUpkeep
        _userDeposits[address(dao)][weth] += arbitrageProfit;  <--------

        // Update the stats related to the pools that contributed to the arbitrage so they can be rewarded proportionally later
        // The arbitrage path can be identified by the middle tokens arbToken2 and arbToken3 (with WETH always on both ends)
        _updateProfitsFromArbitrage( arbToken2, arbToken3, arbitrageProfit );
        }

[Link]

The first arrow in the provided code snippet represents the calculation for the profit of the swap. However, a notable concern is that this calculation does not take into consideration the gas paid for the swap, or at least some specified threshold. It's important to note that this might not be an issue on other chains with low gas fees different from Ethereum.

Impact

The contract may perform arbitrage operations that are not profitable. In some cases, users might pay a significant amount, for instance, 50 USD, for a transaction, while the arbitrage profit is only 1 USD. This situation can lead to users losing interest in using the salty protocol during times of high gas prices.

Proof of Concept

As you can see in the code sniped below the

function _arbitrage(IERC20 arbToken2, IERC20 arbToken3, uint256 arbitrageAmountIn ) internal returns (uint256 arbitrageProfit)
        {
        ...

        // Will revert if amountOut < arbitrageAmountIn
        arbitrageProfit = amountOut - arbitrageAmountIn;   <--------

        ...
        }

The arbitrageProfit is not taking in consideration any gas amount or some predifined threshold.

See the etherium gas tracker, at the time of writing this the average price for a swap is 13.5 usd.

Tools Used

Manual

Recommended Mitigation Steps

Consider implementing a threshold in the profit calculation to compensate for gas fees. This could involve incorporating a fixed threshold that is adjustable by the DAO. Alternatively, a more sophisticated approach could involve gas calculation using available gas opcodes in the EVM. This enhancement would help ensure that users are not incurring excessive fees relative to the arbitrage profits, particularly during periods of elevated gas prices.

Assessed type

Other

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

Within ArbitrageSearch, if no profitable arbitrage is found then it is not done.

The user pays gas for the swap and arbitrage - but the arbitrage cost is simply included in the swap gas cost (which is relatively low).

[Picodes]

There is a dust parameter in the search + the arbitrage profits to the protocol so it's not on the user interest anyway

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid