CodeHawks Escrow Contract - Competition Details

Summary

The contract has an inconsistent use of the nonReentrant modifier using it on the resolveDispute() function but not on the confirmReceipt() function.

Vulnerability Details

Both functions call the safeTransfer method and so, in this case, they either both should have the modifier or neither should. Considering that they follow the CEI pattern, they both have modifiers only allowing specific users to transfer funds and neither of them can be reentered unless using ERC777 which in this case has been stated as a known issue and is discouraged.

Impact

Improves the readability and consistency of the code.

Tools Used

Manual review

Recommendations

Consider either remove the reentrancy guard completely or adding it to all the functions that change state.