Platform: Code4rena
Start Date: 15/06/2022
Pot Size: $35,000 USDC
Total HM: 1
Participants: 36
Period: 3 days
Judge: Jack the Pug
Total Solo HM: 1
Id: 137
League: ETH
Rank: 1/36
Findings: 2
Award: $29,831.82
🌟 Selected for report: 1
🚀 Solo Findings: 1
🌟 Selected for report: 0xDjango
29750 USDC - $29,750.00
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
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.
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.
Manual review.
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.
#0 - jack-the-pug
2022-07-14T02:38:34Z
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.
🌟 Selected for report: 0xNazgul
Also found by: 0xDjango, 0xFar5eer, 0xf15ers, BowTiedWardens, Chom, Dravee, IllIllI, Meera, MiloTruck, PierrickGT, TerrierLover, _Adam, cccz, codexploder, cryptphi, delfin454000, fatherOfBlocks, hansfriese, joestakey, oyc_109, simon135
81.8216 USDC - $81.82
E.g. group all libraries first, then interfaces, then OZ imports
#0 - Yashiru
2022-06-22T15:59:00Z
Quality assurance confirmed