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

[H-01] Lack of emergency withdraw function when no arbiter is set

0xnevi

Impact

If there is no arbiter, buyer can never retract the funds sent to escrow, causing tokens to be lost forever.

In Escrow.initiateDispute(), if no arbiter is set by buyer, dispute can never be initiated.

Escrow.sol#L103

function initiateDispute() external onlyBuyerOrSeller inState(State.Created) {
/// @audit if no arbiter set, dispute cannot be initiated
-> if (i_arbiter == address(0)) revert Escrow__DisputeRequiresArbiter();
s_state = State.Disputed;
emit Disputed(msg.sender);
}

The only way to retrieve back the funds is through buyer/seller first calling initiateDispute() then arbiter calling resolveDispute(). However, Escrow.resolveDispute() will always revert due to the inState modifier because initiateDispute() cannot be called to set state of escrow to Dispute.

Escrow.sol#L109

/// @inheritdoc IEscrow
function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
/// @audit this function will always revert unless initiateDispute is called by buyer or seller
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
uint256 totalFee = buyerAward + i_arbiterFee; // Reverts on overflow
if (totalFee > tokenBalance) {
revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
}
​
s_state = State.Resolved;
emit Resolved(i_buyer, i_seller);
​
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
}
tokenBalance = i_tokenContract.balanceOf(address(this));
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}
}

In the above scenario, the only way to transfer funds out of escrow is for buyer to call confirmReceipt() and send funds to seller.

Proof of Concept

Consider the scenario where there is no arbiter, and buyer is dissatisfied with seller delivery, but escrow contract is created with no arbiter.

In this case, there is no way for buyer to retrieve their funds sent to Escrow contract since both functions initiateDispute() and resolveDispute() cannot be called if there are no arbiter set as their tokens are locked forever in Escrow contract, unless they are willing to go ahead with payment via confirmReceipt().

Tools Used

Manual Analysis

Recommendation

Consider adding an additional onlyBuyer function where withdrawal of escrowed funds by buyer is allowed when there is no arbiter set. However, set a delay so that buyer cannot immediately pull escrowed tokens to grief sellers payment.

In general start of audit to end of audit will normally not take more than 3 months.

Support

FAQs

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