Trick or Treat

The resolveTrick function in the SpookySwap contract is vulnerable to a reentrancy attack due to improper ordering of state updates and external calls. Specifically, it makes an external call to _transfer before updating critical state variables. This flaw allows an attacker to re-enter the contract during the execution of _transfer, potentially manipulating the contract's state or performing unauthorized actions, leading to asset theft or unauthorized access.

Vulnerability Details

• External Call Before State Update: The function calls _transfer, which is an external call that may invoke code in other contracts, especially if msg.sender is a contract. This happens before the critical state variables are updated.

• Reentrancy Risk: If msg.sender is a contract with a malicious onERC721Received function, it can re-enter the SpookySwap contract during the _transfer call.

• State Variables Not Updated: Because the state variables (pendingNFTs, pendingNFTsAmountPaid, tokenIdToTreatName) are not yet updated, the attacker can exploit this window to manipulate the contract's state or call other functions.

Why nonReentrant Modifier May Not Protect:

• The nonReentrant modifier may not be sufficient if the reentrant call occurs through a different entry point or during an external call that invokes untrusted code.

• Reentrancy can still occur if the external call is to an untrusted contract that can call back into the vulnerable function or other functions.

Impact

• Unauthorized Access: Attackers can manipulate the contract's state to gain unauthorized access to NFTs or Ether.

• Asset Theft: Multiple NFTs can be acquired without proper payment, or Ether can be extracted from the contract illicitly.

• State Corruption: The contract's state may become inconsistent, leading to further vulnerabilities or denial of service.