40,000 USDC
View results
Submission Details
Severity: gas
Valid

The computeEscrowAddress() can return an incorrect predicted address

Summary

The EscrowFactory.computeEscrowAddress() can return an incorrect predicted address to an external caller.

Vulnerability Details

The EscrowFactory.computeEscrowAddress() is defined as a public function, allowing both an external caller and the EscrowFactory contract itself (i.e., the newEscrow()) to execute it.

In case of an external call, if a caller inputs the deployer parameter incorrectly, the computeEscrowAddress() will return an incorrect predicted address since the inputted deployer parameter will be used for computing the predicted address.

function computeEscrowAddress(
bytes memory byteCode,
@> address deployer,
uint256 salt,
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) public pure returns (address) {
address predictedAddress = address(
uint160(
uint256(
keccak256(
abi.encodePacked(
bytes1(0xff),
@> deployer,
salt,
keccak256(
abi.encodePacked(
byteCode, abi.encode(price, tokenContract, buyer, seller, arbiter, arbiterFee)
)
)
)
)
)
)
);
return predictedAddress;
}

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/EscrowFactory.sol#L58

https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/EscrowFactory.sol#L73

Impact

The EscrowFactory.computeEscrowAddress() can return an incorrect predicted address if an external caller inputs the deployer parameter incorrectly.

Tools Used

Manual Review

Recommendations

I recommend hard-coding the deployer parameter with the address(this) instead, as shown below.

This will guarantee that no matter whether an external caller or the EscrowFactory contract itself (i.e., the newEscrow()) will execute the computeEscrowAddress(), the function will return a correct predicted address.

function computeEscrowAddress(
bytes memory byteCode,
- address deployer,
uint256 salt,
uint256 price,
IERC20 tokenContract,
address buyer,
address seller,
address arbiter,
uint256 arbiterFee
) public pure returns (address) {
address predictedAddress = address(
uint160(
uint256(
keccak256(
abi.encodePacked(
bytes1(0xff),
- deployer,
+ address(this),
salt,
keccak256(
abi.encodePacked(
byteCode, abi.encode(price, tokenContract, buyer, seller, arbiter, arbiterFee)
)
)
)
)
)
)
);
return predictedAddress;
}

Support

FAQs

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