CodeHawks Escrow Contract - Competition Details

Summary

The addresses for the seller and arbiter are set once upon contract creation. If a mistake is made in the address of either the seller or the arbiters, all funds could be lost.

Vulnerability Details

Both i_seller and i_arbiter are immutables set during the constructor. Additionally, the contract must be funded at or before the time of creation.

Impact

Low. The severity of this is high because the entire contract balance would be lost if a mistake was made in the seller address and there was no arbiter. If a mistake was made in the arbiter address, funds would be lost if the arbiter was needed during the escrow.

The difficulty of triggering this bug is also very high. Although anyone can create an escrow contract, they would have to make a mistake in the address. Because the difficulty is so high, this is a low impact finding in spite of the high severity.

Tools Used

Manual review

Recommendations

Use a two-step process for setting both the seller and arbiter. In the first step, the buyer will propose an address. If the proposed address is not accepted (for example because of a mistake in the address) then the buyer can set a new pending address. Forward immutability can be maintained by adding logic that an address cannot be changed after it is set.

This will increase the runtime gas cost for any functions that read i_seller or i_arbiter but the functions are not commonly called and the security benefits from this mitigation are large.