Currently, all the external calls from the router are unprotected. This means that the executor can pass swapper, flasher, and vault as anything they like and re-enter.
Even if Fuji whitelist swapper, vault, and flasher as solutions to,
there are two legitimate ways users can still re-enter: the flasher callback and withdrawing ETH.
While the flasher reentrancy is intended, the contract currently doesn’t handle it correctly.
It allows the executor to change beneficiary once it's set between the bundle.
To resolve this, consider making beneficiary and tokensToCheck memory variables instead.
Memory for each call is different, unlike storage. Hence, both the parent and child will have their own isolated beneficiary and tokensToCheck. This not only solves the problem but also significantly reduces the gas costs involved due to the use of memory instead of storage.
If you decide to go this route, please add a check inside the "flashloan" action that confirms the first action in requestorCalldata is the same as the one in the parent bundle, similar to how it's done in _crossTransferWithCalldata.
Description
Currently, all the external calls from the router are unprotected. This means that the executor can pass
swapper,flasher, andvaultas anything they like and re-enter.Even if Fuji whitelist
swapper,vault, andflasheras solutions to, there are two legitimate ways users can still re-enter: the flasher callback and withdrawing ETH.While the flasher reentrancy is intended, the contract currently doesn’t handle it correctly. It allows the executor to change beneficiary once it's set between the bundle.
Afected contracts
https://github.com/Fujicracy/fuji-v2/blob/50fd0b74ccee1a73a459118e50e044a2bcfacd10/packages/protocol/src/abstracts/BaseRouter.sol#L75
Resolution
To resolve this, consider making
beneficiaryandtokensToCheckmemory variables instead.Memory for each call is different, unlike storage. Hence, both the parent and child will have their own isolated
beneficiaryandtokensToCheck. This not only solves the problem but also significantly reduces the gas costs involved due to the use of memory instead of storage.If you decide to go this route, please add a check inside the "flashloan" action that confirms the first action in
requestorCalldatais the same as the one in the parent bundle, similar to how it's done in_crossTransferWithCalldata.