search

Cyfrin / 2023-07-escrow

16 stars 12 forks source link

Buyer or Seller and arbiter can be set to the same address #832

Open codehawks-bot opened 1 year ago

codehawks-bot commented 1 year ago

Buyer or Seller and arbiter can be set to the same address

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L32-L51

Summary

Buyer or Seller can be set to the same address as the arbiter. The buyer or the seller who is also arbiter can call resolveDispute freely.

Vulnerability Details

In the Escrow contract constructor, it does not check if the buyer or seller is the same as the arbiter. If the buyer or seller is set to arbiter, the arbiter is no longer neutral.

When initiateDispute is called and a dispute arises, and the buyer or seller is arbiter, then buyer or seller can freely call resolveDispute to distribute tokens as desired.

constructor(
    uint256 price,
    IERC20 tokenContract,
    address buyer,
    address seller,
    address arbiter,
    uint256 arbiterFee
) {
    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;
}

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L32-L51

Impact

The buyer or the seller who is also arbiter can call resolveDispute freely.

Tools Used

vscode

Recommendations

In the constructor, check that the arbiter is neither a buyer nor a seller.