CodeHawks Escrow Contract - Competition Details

Summary

A possible gas optimization by removing an error check that is seems unnecessary or is placed in the wrong contract.

Vulnerability Details

The following error (if (buyer == address(0)) revert Escrow__BuyerZeroAddress();) in the Escrow.sol contract is verifying if the buyer address provided by the Escrow Factory contract as msg.sender is zero address or not. In practical scenarios msg.sender can not be equal to zero address but considering an exceptional case that somehow an attacker can get a condition that buyer is equal to zero address. Even then the transaction will not be successful because Escrow Factory contract transfers tokens before creating the contract and since transferring ERC20 tokens requires approval to transfer tokens, the zero address would need approve this operation, which is not possible because it can't sign transactions. Hence, the line of code tokenContract.safeTransferFrom(msg.sender, computedAddress, price); would certainly fail.

Impact

Removing or transferring this error check could lead to a decrease in gas consumption and increase in efficiency during contract execution, as error checks consume gas.

Tools Used

1. VS Code

2. Hardhat

3. Manual Review

Recommendations

1. Consider relocating this error to Escrow Factory contract from Escrow contract.

2. Remove the error altogether.