search

Cyfrin / 2023-07-escrow

17 stars 12 forks source link

Funds can be stuck in contract if deployed #747

Open codehawks-bot opened 11 months ago

codehawks-bot commented 11 months ago

Funds can be stuck in contract if deployed

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L32-L51

Summary

If the contract is erroneously deployed with a zero arbiter address and has a dispute it cannot be settled and funds cannot be withdrawn as disputes cannot be initiated

Vulnerability Details

The contract can be successfully deployed with a zero arbiter address, which shouldn't be allowed, it implements Zero address Checks for the seller and buyer but not the arbiter, this completely removes any possibility of Dispute initiation and resolution and funds sent to the contract have been lost forever.

Impact

Deployer/Buyer funds can get stuck in contract if arbiter address is erroneously a zero address.

Tools Used

Manual Review

Recommendations

There should be zero address checks in the constructor of the Escrow.sol to prevent deployment in case of zero arbiter address

if (arbiter == address(0)) revert Escrow__ArbiterZeroAddress();
PatrickAlphaC commented 10 months ago

Arbitor can be the zero address. That's the risk the two take if the land on no arbitor

PatrickAlphaC commented 10 months ago

If you gave the scenario of a malicious buyer I would have given it to you