sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

volodya - Its possible to create multiple swappers with the same params #8

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

volodya

medium

Its possible to create multiple swappers with the same params

Summary

According to uniswap whitepaper its not possible to create two pools with the same params.

In Uniswap v1 and v2, every pair of tokens corresponds to a single liquidity pool, which applies a uniform fee of 0.30% to all swaps. While this default fee tier historically worked well enough for many tokens, it is likely too high for some pools (such as pools between two stablecoins), and too low for others (such as pools that include highly volatile or rarely traded tokens).

Vulnerability Detail

In the current implementation, SwapperFactory allows creating a swapper with the same params which are opposite to what uniswap is intentionally avoiding due to security issues.


function createSwapper(CreateSwapperParams calldata params_) external returns (SwapperImpl swapper) {
OracleImpl oracle = params_.oracleParams._parseIntoOracle();
    swapper = SwapperImpl(payable(address(swapperImpl).clone()));
    SwapperImpl.InitParams memory swapperInitParams = SwapperImpl.InitParams({
        owner: params_.owner,
        paused: params_.paused,
        beneficiary: params_.beneficiary,
        tokenToBeneficiary: params_.tokenToBeneficiary,
        oracle: oracle
    });
    swapper.initializer(swapperInitParams);
    $isSwapper[swapper] = true;

    emit CreateSwapper({swapper: swapper, params: swapperInitParams});
}
[splits-swapper/src/SwapperFactory.sol#L41](https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperFactory.sol#L41)
## Impact
I believe the whole system will be comprised otherwise uniswap will not be forbidding to create pools v3 and pairs in v2 with the same params
## Code Snippet

## Tool used

Manual Review

## Recommendation
Use salt to create a key for input params and restrict to create with the same key
```solidity
    function deploy(
        address factory,
        address token0,
        address token1,
        uint24 fee,
        int24 tickSpacing
    ) internal returns (address pool) {
        parameters = Parameters({factory: factory, token0: token0, token1: token1, fee: fee, tickSpacing: tickSpacing});
        pool = address(new UniswapV3Pool{salt: keccak256(abi.encode(token0, token1, fee))}());
        delete parameters;
    }

etherscan

Code from v2 with salt

    function createPair(address tokenA, address tokenB) external returns (address pair) {
        require(tokenA != tokenB, 'UniswapV2: IDENTICAL_ADDRESSES');
        (address token0, address token1) = tokenA < tokenB ? (tokenA, tokenB) : (tokenB, tokenA);
        require(token0 != address(0), 'UniswapV2: ZERO_ADDRESS');
        require(getPair[token0][token1] == address(0), 'UniswapV2: PAIR_EXISTS'); // single check is sufficient
        bytes memory bytecode = type(UniswapV2Pair).creationCode;
        bytes32 salt = keccak256(abi.encodePacked(token0, token1));
        assembly {
            pair := create2(0, add(bytecode, 32), mload(bytecode), salt)
        }
        IUniswapV2Pair(pair).initialize(token0, token1);
        getPair[token0][token1] = pair;
        getPair[token1][token0] = pair; // populate mapping in the reverse direction
        allPairs.push(pair);
        emit PairCreated(token0, token1, pair, allPairs.length);
    }