CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Zero Arbiter Puts Funds at Risk of Unresolved Disputes!

devanas

Summary

The Escrow contract has a critical vulnerability that allows the arbiter address to be set to the zero address during deployment, leaving the contract without a valid and impartial arbiter to mediate disputes. Additionally, due to the immutability of the i_arbiter variable, the dispute function initiateDispute() can only be called if the i_arbiter is not zero, making it inaccessible to both the buyer and seller.

Vulnerability Details

  1. Missing Arbiter:
    During contract deployment, the arbiter parameter is set, and it can be assigned the zero address (0x0000000000000000000000000000000000000000). This results in a missing or invalid arbiter, leaving disputes unresolved and compromising the integrity of the escrow arrangement.

  2. Restricted Dispute Function:
    The initiateDispute() function in the Escrow contract checks whether the i_arbiter address is not the zero address before allowing dispute initiation. Since the i_arbiter address is initialized during contract deployment and is immutable, both the buyer and seller cannot update or change it to a valid address, rendering them unable to call the initiateDispute() function.

Here's the code example:

address sameAddress = vm.addr(1);
address public constant ARBITER_ZERO = address(0);
function testWhenBuyerAndSellerAndArbiterAreSame() public {
vm.startPrank(sameAddress);
ERC20Mock(address(i_tokenContract)).mint(sameAddress, PRICE);
ERC20Mock(address(i_tokenContract)).approve(
address(escrowFactory),
PRICE
);
escrow = escrowFactory.newEscrow(
PRICE,
i_tokenContract,
sameAddress,
ARBITER_ZERO,
ARBITER_FEE,
SALT1
);
vm.stopPrank();
console.log("Arbiter addresss -----------", ARBITER_ZERO);
assertEq(escrow.getPrice(), PRICE);
assertEq(address(escrow.getTokenContract()), address(i_tokenContract));
assertEq(escrow.getBuyer(), sameAddress);
assertEq(escrow.getSeller(), sameAddress);
assertEq(escrow.getArbiter(), ARBITER_ZERO);
assertEq(escrow.getArbiterFee(), ARBITER_FEE);
}

Impact

The vulnerability has significant consequences:

  • Lack of a valid arbiter jeopardizes dispute resolution, leading to potential conflicts and financial risks for both parties.

  • The restriction on the dispute function prevents the buyer and seller from initiating dispute resolutions, limiting their ability to resolve conflicts independently.

Tools Used

Manual Review

Recommendations

Ensure that the arbiter address is properly validated during contract deployment to prevent setting it to the zero address.

We can add the check effect in constructor:

if (i_arbiter == address(0)) revert Escrow__ArbiterZeroAddress();

Since we are already checking that the arbiter should not be the zero address during contract deployment and it is an immutable address that cannot be updated afterward, we can remove the if statements in the initiateDispute() function for gas optimization. This will enhance the efficiency of the contract execution and reduce unnecessary checks.

Support

FAQs

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