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

Potential loss of fund due to the oversight of seller being equal to arbiter address

0xpep7

Summary

The oversight of the seller being equal to the arbiter could potentially enable the seller to claim all the funds through early dispute resolution without fulfilling their service obligation, leading to a loss of funds for the buyer.

Vulnerability Details

The vulnerability exists in the constructor function of Escrow.sol, specifically at line 48. The constructor fails to account for the scenario where the seller address is equal to the arbiter address. This oversight could be exploited by the seller to initiate an early dispute resolution and claim all the funds without providing the agreed-upon service.

Vulnerable Code Snippet:

constructor(address tokenContract, address buyer, address seller, address arbiter, uint256 price, 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;
// @audit-issue seller == arbiter is overlooked. This could lead to seller immediately claiming all the fund via early dispute and resolve without performing their service. Loss of fund for buyer should be prevented.
i_seller = seller;
i_arbiter = arbiter;
i_arbiterFee = arbiterFee;
}

Impact

The oversight of allowing the seller and arbiter addresses to be the same creates a severe vulnerability that benefits the seller. By initiating an early dispute and resolving it in their favor, the seller can claim all the funds without fulfilling their service obligations. Consequently, the buyer will suffer a complete loss of funds.

Tools Used

Manual Review

Recommendations

To mitigate this vulnerability and prevent the seller from unfairly claiming all the funds without providing their service, it is crucial to add a validation check in the constructor. This check should ensure that the seller address is not the same as the arbiter address.

Recommended Modification:

constructor(address tokenContract, address buyer, address seller, address arbiter, uint256 price, 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();
+ require(seller != arbiter, "Seller and arbiter cannot be the same address");
i_price = price;
i_tokenContract = tokenContract;
i_buyer = buyer;
i_seller = seller;
i_arbiter = arbiter;
i_arbiterFee = arbiterFee;
}

Support

FAQs

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