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

Zero Price Vulnerability in `EscrowFactory.sol` Contract

0xNiloy

Summary

The newEscrow function in the EscrowFactory contract does not have any checks against a zero price. This could potentially allow a user to create an escrow contract without depositing any funds. While this might not necessarily be a vulnerability, it could lead to unexpected behavior and should be considered in the design of the contract.

Vulnerability Details

The newEscrow function is designed to create a new instance of the Escrow contract. It accepts several parameters, including a price parameter that represents the amount of funds to be deposited into the escrow contract. However, there is no explicit check in the function to prevent the price from being set to zero. This means that a user could potentially create a new escrow contract without depositing any funds.

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 with a zero price, it could lead to unexpected behavior. For example, the seller might not be aware that no funds have been deposited and could proceed with providing the service under the assumption that they will be paid. This could potentially lead to disputes and loss of 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 price is greater than zero. This could be implemented as a simple require statement, like so:

require(price > 0, "Price must be greater than zero");

This would ensure that an escrow contract cannot be created without depositing any funds, thereby preventing the potential issues described above.

Support

FAQs

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