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

If no arbiter is set during deployment, no dispute can be initiated setting user funds in danger.

0xl3xx

Summary

Not setting an arbiter during escrow construction results in not being able to make a dispute since the buyer can not set/change the arbiter after deployment putting buyer funds at risk.

Vulnerability Details

When constructing an escrow a team may choose that they do not want an arbiter. So, if there is no arbiter set during construction, neither the seller nor the buyer can initiateDispute().

This is shown in the test case below:

function testInitiateDisputeWithoutArbiterReverts() public {
vm.startPrank(BUYER);
ERC20Mock(address(i_tokenContract)).mint(BUYER, PRICE);
ERC20Mock(address(i_tokenContract)).approve(address(escrowFactory), PRICE);
escrow = escrowFactory.newEscrow(PRICE, i_tokenContract, SELLER, address(0), ARBITER_FEE, SALT1);
vm.expectRevert(IEscrow.Escrow__DisputeRequiresArbiter.selector);
escrow.initiateDispute();
vm.stopPrank();
}

This way the funds may be stuck in the contract or the buyer can only confirmReceipt() and send the funds to the seller despite the dispute that exists.

In fact the i_arbiter is immutable so it can not be changed after contract deployment and there is no function that can change the arbiter, if it is address(0).

Impact

Disputing an escrow is a core functionality of the contract and the absence of an arbiter causes the protocol to not work as intended and also the funds are at stake since the buyer either leaves their funds on the protocol or sends them to the seller despite the dispute by confirming the receipt of the report.

Tools Used

Manual code review

Recommendations

Consider removing the immutable property from i_arbiter and add a setter function that can be called by buyers or not permitting the zero address for arbiters like during escrow construction like so:

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

Support

FAQs

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