Open code423n4 opened 2 years ago
I find this to be a valid Medium issue. I have a few things to add:
entryFee
, the exitFees
can also be bypassed in a similar way;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.
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 usingNestedFactory.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 anentryFee
of0
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
NestedFactory.create()
with a single input order. This input order will define the parameters of the call to Paraswap.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.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: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.