CodeHawks Escrow Contract - Competition Details

Summary

Vulnerability Details

Use abi.encode() instead which will pad items to 32 bytes, which will
prevent hash collisions
(e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456),
but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason,
abi.encode should be preferred". If there is only one argument to abi.encodePacked()
it can often be cast to bytes() or bytes32() instead.

There is also discussion of removing abi.encodePacked from future versions of Solidity,
so using abi.encode now will ensure compatibility in the future.

There is 1 instance of this issue:

• EscrowFactory.computeEscrowAddress(bytes,address,uint256,uint256,IERC20,address,address,address,uint256) calls abi.encodePacked() with multiple dynamic arguments:

  • predictedAddress = address(uint160(uint256(keccak256(bytes)(abi.encodePacked(bytes1(0xff),deployer,salt,keccak256(bytes)(abi.encodePacked(byteCode,abi.encode(price,tokenContract,buyer,seller,arbiter,arbiterFee))))))))

Impact

Tools Used

Recommendations

Do not use more than one dynamic type in abi.encodePacked()
(see the Solidity documentation).
Use abi.encode(), preferably.