hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

GNU General Public License v3.0
0 stars 0 forks source link

Duplicate Pools Can Be Created with Identical Tokens and Parameters Using `createSwapPair` #83

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xa0f76ed3f1cf016f8801f838b4ee6e099841950bfd2e45dac7c58f4d80d7a01a Severity: medium

Description: Description\ The createSwapPair function in the StableSwapTwoPoolDeployer contract is protected by the onlyOwner modifier, meaning it can only be called by the contract owner.

However, despite this restriction, there is no mechanism in place to prevent the contract owner from unintentionally creating multiple stable swap pools with identical token pairs and parameters (amplification coefficient _A, fees _fee and _admin_fee).

The function allows multiple identical pools to be deployed because a unique salt is generated using block.timestamp, which changes with each call, even if the tokens and parameters remain the same.

Attack Scenario\ The owner, either through operational error or oversight, might deploy the same pool multiple times, with the same token pair and parameters. This would result in Liquidity Fragmentation and Arbitrage Opportunities

Recommendation


+ mapping(bytes32 => address) public swapPairRegistry;

function createSwapPair(
        address _tokenA,
        address _tokenB,
        uint256 _A,
        uint256 _fee,
        uint256 _admin_fee,
        address _admin,
        address _LP
    ) external onlyOwner returns (address) {
        require(_tokenA != address(0) && _tokenB != address(0) && _tokenA != _tokenB, "Illegal token");
        (address t0, address t1) = sortTokens(_tokenA, _tokenB);
+bytes32 poolKey = keccak256(abi.encodePacked(t0, t1, _A, _fee, _admin_fee));

+require(swapPairRegistry[poolKey] == address(0), "Pool already exists for this pair");
        address[N_COINS] memory coins = [t0, t1];
        // create swap contract
        bytes memory bytecode = type(StableSwapTwoPool).creationCode;
        bytes32 salt = keccak256(abi.encodePacked(t0, t1, msg.sender, block.timestamp, block.chainid));
        address swapContract;
        assembly {
            swapContract := create2(0, add(bytecode, 32), mload(bytecode), salt)
        }
        StableSwapTwoPool(swapContract).initialize(coins, _A, _fee, _admin_fee, _admin, _LP);

+swapPairRegistry[poolKey] = swapContract;

        return swapContract;
    }

The fix introduces a pool registry (swapPairRegistry) that stores deployed pool addresses using a unique key generated from the token pair and parameters.

The require statement ensures that a pool with the same parameters cannot be deployed multiple times by mistake, even by the contract owner.

This fix prevents accidental duplicate pool deployments, ensuring that liquidity remains consolidated and economic inefficiencies are avoided.

Ghoulouis commented 2 weeks ago

It's a duplicate of https://github.com/hats-finance/Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d/issues/18