Summary

Note: Submitting this as informational to ensure this is known despite this feature being by design. The gist of it is: if no arbiter is set and something happens to the auditor mid-audit, there's no way for the protocol to recover his tokens.

Vulnerability Details

The escrow contract allows for the arbiter to be address(0). If no arbiter is set, then the dispute process can't be started. This means that in the case where an arbiter is not set, only confirmReceipt can be called due to this check:

https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L103

Although this is by design, it introduces the edge case where if arbiter == address(0) and the auditor doesn't perform the audit, it would leave the client without the possibility of claiming the tokens he escrowed back. The only thing he can do in this situation is call confirmReceipt and transfer the tokens to the auditor (leaving the client at the auditor's good will--or even helpless if the auditor lost control of his account) or leave them locked in the escrow contract forever.

Small PoC:

• Protocol deploys an escrow contract, sending 1000 tokens.

• No arbiter is set.

• Auditor doesn't perform the audit for some reason. The only ones I can come up with are extreme, such as passing away.

• Protocol attempts to dispute by calling initiateDispute. It reverts due to no arbiter.

• Protocol tokens are essentially locked.

Impact

Buyer funds, which are equal to the price the client is willing to pay for an audit, can be potentially irrecoverable.

Recommendations

Enforce at the constructor level that the arbiter can't be address(0).