Submitted by kirk-baird, also found by ACai, hake, and rayn
DiamondCutFacet.sol#L14-L22
CBridgeFacet.sol#L92-L121
AnyswapFacet.sol#L74-L110
NXTPFacet.sol#L85-L102
NXTPFacet.sol#L150-L171
There is a reenterancy vulnerability in functions which call Swapper._executeSwap() which would allow the attacker to change their postSwapBalance.
The functions following similar logic to that seen in GenericSwapFacet.swapTokensGeneric().
This logic records the balance before and after the _executeSwaps() function. The difference is then transferred to the msg.sender.
The issue occurs since it is possible for an attacker to reenter this function during _executeSwaps(), that is because execute swap makes numerous external calls, such as to the AMM, or to untrusted ERC20 token addresses.
If a function is called such as WithdrawFacet.withdraw() this will impact the calculations of postSwapBalance which will account for the funds transferred out during withdrawal. Furthermore, any functions which transfers funds into the contract will also be counted in the postSwapBalance calculations.
Vulnerable Functions:
GenericSwapFacet.swapTokensGeneric()
CBridgeFacet.swapAndStartBridgeTokensViaCBridge()
AnyswapFacet.swapAndStartBridgeTokensViaAnyswap()
HopFacet.swapAndStartBridgeTokensViaHop()
NXTPFacet.swapAndStartBridgeTokensViaNXTP()
NXTPFacet.swapAndCompleteBridgeTokensViaNXTP()
Consider adding a reentrancy guard over every function which may send or receive tokens. It may be easiest too add this guard over the fallback() function however that could prevent view functions from being called (since it would perform storage operations).
Ensure the same slot is used to store the reentrancy guard so all required functions are covered by a single guard.
H3xept (Li.Fi) commented:
H3xept (Li.Fi) resolved and commented:
gzeon (judge) commented:
