Summary

The escrow contract contains a reentrancy vulnerability that allows an attacker to withdraw tokens during the deposit process, potentially leading to unauthorized access and theft of assets.

Vulnerability Details

The _withdrawFromEscrow function lacks proper reentrancy protection. This flaw allows an attacker to call the withdrawFromEscrow function during the execution of the depositIntoEscrow function, leading to unexpected behavior and unauthorized withdrawal of tokens.

Impact

An attacker can exploit this vulnerability to drain assets from the escrow contract by re-entering the contract during the deposit phase. This could result in significant financial loss for the users of the contract.

Tools Used

Manual Review

PoC (Proof of Concept)

Place this test in your Escrow.t.sol file :

1. Minting Tokens: Alice mints 10 ERC721 tokens (IDs 0-9) to her account.

2. Prepare Tokens for Deposit: Alice prepares two token IDs (5 and 8) for deposit into the escrow contract.

3. Start Prank: The testing framework's prank feature is activated to simulate Alice's actions.

4. Deposit Tokens: Alice deposits the selected tokens (IDs 5 and 8) into the escrow contract.

5. Reentrancy Exploit: During the deposit, the withdrawFromEscrow function is called to withdraw token ID 5 to Bob's account.

6. Result: Token ID 5 is transferred to Bob, showing that an unauthorized withdrawal occurred during the deposit.

Recommendations

1. To mitigate this vulnerability, implement a reentrancy guard on the withdrawFromEscrow function. This can be done by using the nonReentrant modifier provided by libraries like OpenZeppelin or by implementing a custom reentrancy guard to ensure that no reentrant calls can be made during sensitive operations.

2. Use a CEI(Checks Effects Interactions) Pattern for writing code.