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

Self-Dealing Vulnerability in `EscrowFactory.sol` Contract

0xNiloy

Summary

The newEscrow function in the EscrowFactory contract does not have any checks against the seller and buyer being the same address. This could potentially allow a user to create an escrow contract where they are both the buyer and the seller, which could lead to unexpected behavior.

Vulnerability Details

The newEscrow function is designed to create a new instance of the Escrow contract. It accepts several parameters, including a seller and buyer parameter that represent the addresses of the seller and buyer respectively. However, there is no explicit check in the function to prevent the seller and buyer from being the same address. This means that a user could potentially create a new escrow contract where they are both the buyer and the seller.

Code Snippet

function newEscrow(
uint256 price,
IERC20 tokenContract,
address seller,
address arbiter,
uint256 arbiterFee,
bytes32 salt
) external returns (IEscrow) {
address computedAddress = computeEscrowAddress(
type(Escrow).creationCode,
address(this),
uint256(salt),
price,
tokenContract,
msg.sender,
seller,
arbiter,
arbiterFee
);
tokenContract.safeTransferFrom(msg.sender, computedAddress, price);
Escrow escrow = new Escrow{salt: salt}(
price,
tokenContract,
msg.sender,
seller,
arbiter,
arbiterFee
);
if (address(escrow) != computedAddress) {
revert EscrowFactory__AddressesDiffer();
}
emit EscrowCreated(address(escrow), msg.sender, seller, arbiter);
return escrow;
}

Impact

If a user is able to create an escrow contract where they are both the buyer and the seller, it could lead to unexpected behavior. For example, the user could potentially manipulate the contract to their advantage, such as by initiating a dispute and then resolving it in their favor. This could potentially undermine the trust in the platform.

Tools Used

Manual code review

Recommendations

To mitigate this potential issue, it is recommended to add a check in the newEscrow function to ensure that the seller and buyer are not the same address. This could be implemented as a simple require statement, like so:

require(seller != buyer, "Seller and buyer must be different addresses");

This would ensure that an escrow contract cannot be created where the seller and buyer are the same address, thereby preventing the potential issues described above.

Support

FAQs

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