User can bypass entryFee by sending arbitrary calldata to ParaSwap operator

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L466
https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/operators/Paraswap/ParaswapOperator.sol#L34

Vulnerability details

Impact

Any user is able to bypass the entryFee collection when using NestedFactory.create() by passing in arbitrary calldata when using the ParaSwap router. High level, a user can pass in calldata to swap from a miniscule amount of input token to an ERC777 with themselves as the recipient and will gain control of execution, at which time they can send a large amount of output token back to the Nested Factory.

If the user sends 1 wei of input token, the Nested Factory will return an entryFee of 0 due to precision loss. The amount of output token returned to the contract via the direct transfer from the user will then be deposited in the vault.

Proof of Concept

Steps

• User calls NestedFactory.create() with a single input order. This input order will define the parameters of the call to Paraswap.
• The single order defines the following in pseudocode:
  1. inputToken: Any token, but we'll use address(0) ETH
  2. amount: 1 wei
  3. Order(operator=Paraswap, token=USDC, calldata=calldata)

The calldata used in the call to paraswap would swap from ETH to any ERC777 (NOT USDC), with an attack contract address set as the beneficiary. Upon transferring the swapped ERC777 to the user's attack contract, the contract would immediately send e.g. 1,000,000 USDC directly back to the Nested Factory contract.

• The Paraswap operator checks the balances of the buy and sell tokens. Note that the buy token is defined in the Order token parameter, not the calldata passed to Paraswap. Since the operator will check the balance of USDC, it looks like we've swapped 1 wei ETH for 1,000,000 USDC.
• The Paraswap operator returns the swap amounts back to Nested Factory.
• Nested Factory deposits the 1,000,000 USDC to the vault for the user without charging any entryFee.

NOTE: I use 1 wei as an extreme example. You would have to ensure that you're swapping at least enough to receive 1 wei of the ERC777 to transfer to the attack contract.

Tools Used

Manual review.

Recommended Mitigation Steps

Allowing a user to pass arbitrary call data to a router is risky because routers allow several paths for an attacker to gain control of execution. Originally, I believed this exploit to be possible simply by swapping to ETH, which would perform an external call to the beneficiary, but Paraswap actually only forwards 10,000 gas when performing ETH transfers. If Nested plans to include a vanilla Uniswap router operator, this would be an issue. Here is the Paraswap transfer logic:

function transferTokens(
        address token,
        address payable destination,
        uint256 amount
    )
    internal
    {
        if (amount > 0) {
            if (token == ETH_ADDRESS) {
                (bool result, ) = destination.call{value: amount, gas: 10000}("");
                require(result, "Failed to transfer Ether");
            }
            else {
                IERC20(token).safeTransfer(destination, amount);
            }
        }

    }

Therefore, it might be worth exploring the option of allowing the user to only choose from a list of predefined function signatures when making calls to Paraswap. The final Order param that is passed to the operator would be built within the contract by concatenating the function, input, and output tokens. Even then, if the output token truly is an ERC777, the user would be able to intercept control and directly transfer more of the ERC777.

A large-scale fix would be to charge the entry fee on the amount of output tokens after performing the swap. I'm not sure if this falls in line with Nested's plans though.

[jack-the-pug]

I find this to be a valid Medium issue. I have a few things to add:

1. It applies not only to the ParaSwap operator but also the other operators, ie, 0x;
2. Not just the entryFee, the exitFees can also be bypassed in a similar way;
3. Not necessarily using a ERC777, the attacker can also use a malicious ERC20 they deployed on their own;

The root cause is that:

entryFee and exitFees should be charged on the token that gets in and out the Reserve, not the inputToken/outputToken of the swap.