c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/utils/Swapper.sol#L172 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/utils/Swapper.sol#L161 https://github.com/Uniswap/v3-periphery/blob/main/contracts/libraries/PoolAddress.sol#L33

Vulnerability details

Impact

A malicious user can steal tokens from the Swapper by providing data with a fake token

Proof of Concept

Swapper contract accept callbacks from uniswap pools where amount for swap is payed:

    function uniswapV3SwapCallback(int256 amount0Delta, int256 amount1Delta, bytes calldata data) external override {
        require(amount0Delta > 0 || amount1Delta > 0); // swaps entirely within 0-liquidity regions are not supported

        // check if really called from pool
        (address tokenIn, address tokenOut, uint24 fee) = abi.decode(data, (address, address, uint24));
@>        if (address(_getPool(tokenIn, tokenOut, fee)) != msg.sender) {
            revert Unauthorized();
        }

        // transfer needed amount of tokenIn
        uint256 amountToPay = amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta);
        SafeERC20.safeTransfer(IERC20(tokenIn), msg.sender, amountToPay);
    }

It tries to verify that a call was made from the real pool by calling _getPool and calculate the real address:

    function _getPool(address tokenA, address tokenB, uint24 fee) internal view returns (IUniswapV3Pool) {
        return IUniswapV3Pool(PoolAddress.computeAddress(address(factory), PoolAddress.getPoolKey(tokenA, tokenB, fee)));
    }

The problem is it never verifies that the pool exist, but only tries to get the address:

    struct PoolKey {
        address token0;
        address token1;
        uint24 fee;
    }

    function getPoolKey(
        address tokenA,
        address tokenB,
        uint24 fee
    ) internal pure returns (PoolKey memory) {
        if (tokenA > tokenB) (tokenA, tokenB) = (tokenB, tokenA);
        return PoolKey({token0: tokenA, token1: tokenB, fee: fee});
    }

    function computeAddress(address factory, PoolKey memory key) internal pure returns (address pool) {
        require(key.token0 < key.token1);
        pool = address(
            uint160(
                uint256(
                    keccak256(
                        abi.encodePacked(
                            hex'ff',
                            factory,
                            keccak256(abi.encode(key.token0, key.token1, key.fee)),
                            POOL_INIT_CODE_HASH
                        )
                    )
                )
            )
        );
    }

So a malicious user can calculate and delopy the contract that will match the calculated address with the information about a token he wants to steal and a fake token.

It will require some serious computaions for the hacker to break the right hash and build the valid address, but it is still possible with modern tools.

Tools Used

Manual review

Recommended Mitigation Steps

Verify with the factory that msg.sender is a valid pool

Assessed type

Invalid Validation

c4-pre-sort commented 5 months ago

0xEVom marked the issue as duplicate of #188

c4-pre-sort commented 5 months ago

0xEVom marked the issue as insufficient quality report

0xEVom commented 5 months ago

Insufficient proof

c4-judge commented 5 months ago

jhsagd76 changed the severity to QA (Quality Assurance)