Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: high

Late checking of arbiter in Escrow Contract.

BernardC

Summary

If the arbiter is not able to confer both parties offchain, then it's not an escrow contract and the buyer can directly send funds to the seller once goods or services have been delivered. Therefore, it's essential to verify and input the correct arbiter address.

The initiateDispute() (line 102) function in the Escrow.sol contract should check if the arbiter contract is not a Zero Address and a contract address during the contract creation phase in the constructor call. We can't afford to check the arbiter address is a Zero address when there is a dispute , after the token has been transfer to the esrow contract.

Expected dispute workflow

  1. The buyer creates an Escrow contract through EscrowFactory::newEscrow, depositing funds.

  2. Either party can initiate a dispute through Escrow::initiateDispute.

  3. The arbiter confers with both parties offchain and calls Escrow::resolveDispute to reimburse either side accordingly, emptying the escrow.

Vulnerability Details

function initiateDispute() external onlyBuyerOrSeller inState(State.Created) {
if (i_arbiter == address(0)) revert Escrow__DisputeRequiresArbiter();
s_state = State.Disputed;
emit Disputed(msg.sender);
}

Impact

Failure to check for a Zero Address Arbiter will render the Escrow Contract useless as tokens can only be transferred out of it by confirming receipt without any dispute from buyers.

Tools Used

Slither.

Recommendations

Add two additional checks in the Escow.sol construction function:

  1. Declare a new function in Escow.sol called isContract() which checks if an address is a contract address:

function isContract(address _address) public view returns (bool) {
return _address.code.length > 0;
}

  1. Add custom error code for Arbiter Zero Address in IEscrow.sol:

interface IEscrow {
// Errors
...
error Escrow__ArbiterZeroAddress();
}

  1. Modify the constructor function in Escow.sol to include the new checks for Zero Address and Externally Owned Accounts (EOA):

constructor(
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) {
if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
if (seller == address(0)) revert Escrow__SellerZeroAddress();
// NEW CHECK: Verify that the arbiter is not a Zero Address.
if (arbiter == address(0)) revert Escrow__ArbiterZeroAddress();
​
// NEW CHECK: Verify that the arbiter is an Externally Owned Account.
require(isContract(arbiter) != true, 'Arbiter must be an EOA.');
​
...
​
}

  1. Declare a new view function for the arbiter to get the maximum amount to refund back to the buyer:

function MaxBuyerAward() public view returns(uint256) {
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
return tokenBalance - i_arbiterFee;
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.