CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: low

Arbitor fee could be excessively set

bauchibred

Summary

The current implementation of the Escrow contract allows an arbiter's fee to be set to even as high as 99% of the price, which seems to be an unnecessarily high percentage allowe.

Vulnerability Details

In the provided Escrow.sol contract, the Escrow constructor includes a check to prevent the arbiter's fee from exceeding the transaction price:

if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);

However, the problem with this implementation is that it only checks if the arbiter's fee is equal to or greater than the price, but it does not limit the proportion of the price that the arbiter's fee can be. This means that the arbiter's fee can be set to 99% of the price, which could be seen as unreasonably high for an escrow service.

As indicated in the comment with the "@audit" tag:

constructor(
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) {
// @audit if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee) doesn't make sense if fee is 99% of price, instead a particular amount of fee genereally should be set vor arbiters, say it can't be more than 50% cause as at present implementation fee can get set to as high as 99%
if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
if (seller == address(0)) revert Escrow__SellerZeroAddress();
if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
i_price = price;
i_tokenContract = tokenContract;
i_buyer = buyer;
i_seller = seller;
i_arbiter = arbiter;
i_arbiterFee = arbiterFee;
}

Impact

Allowing an excessively high arbiter's fee seems like the wrong logic to use

Tools Used

Manual Audit

Recommended Mitigation

Consider implementing a restriction on the maximum allowable proportion of the price that can be set as the arbiter's fee. For example, a limitation where the arbiter's fee cannot be more than 50% of the price could be deemed more reasonable.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.