CodeHawks Escrow Contract - Competition Details

Summary

arbiterFee and buyerAward should be a percentage of price

Vulnerability Details

using a fixed fee for arbiter is not very convenient for the contract and its payment calculations. Percentages are more intuitive and easier to work with in terms of ensuring sanity checks for values

Impact

Gas Savings + Informational :

1. Improve the code readability and maintanability

2. Fee percentage is more intuitive and comparable as opposed to amounts

3. May make computations and code cleaner

4. Allows to Improve code to have a check that e.g sellerAwardPercent + buyerAwardPercent + arbiterAwardPercent = 100%; so that arbiter only needs to input in function resolveDispute(uint256 buyerAwardPercent)

Tools Used

Manual Analysis

Recommendations

It is recommended arbiterFee is a percentage agreed beforehand and replaces arbiterFee
function newEscrow(
uint256 price,
IERC20 tokenContract,
address seller,
address arbiter,
uint256 arbiterFee,// replace with arbiterFeePercent e.g 5%
bytes32 salt
) external returns (IEscrow) { ...sanity check arbiterFee > 0 and && < threshold e.g 50% as dont want arbiter getting more than half of the money always it does not make for good incentives for buyers and sellers

Change resolveDispute in Escrow.sol line 109 to the below
function resolveDispute(uint256 buyerPercent) external onlyArbiter nonReentrant inState(State.Disputed) {
.... check buyerPercent + arbiterFeePercent < 100 (if ==100 seller not receiving any remnant, else if <100 there is remnant for seller
..adjust remaining payment logic accordingly bases on amounts calculated by percentages