CodeHawks Escrow Contract - Competition Details

Summary

Currently, tokens are pushed to the intended recipients using safeTransfer. Instead, a pull payment method should be used whereby the intended recipients can pull and receive the payments e.g., with a withdraw function

Vulnerability Details

In both confirmReceipt and resolveDispute, funds are pushed i.e., transferred to the buyer, arbiter and seller. This is particularly an issue in resolveDispute, where there are three instances of safeTransfer sending tokens to the buyer (the buyerAward), arbiter (arb fee) and (whats left) to seller.

With this change, there would be no need to use the nonReentrant contract, thereby improving the security outlook and saving on considerable gas, both on deployment and per function call.

Impact

Gas

Tools Used

Forge, Foundry Toolkit (gas report, gas snapshots)

Recommendation

Refactor push payment logic to pull payment. By introducing with draw functions and removing push payments from resolveDispute, a gas saving of around 9000 gas was seen on construction tests. However, other tests got expensive; this is expected as the gas burden shifts over to the withdrawer.