CodeHawks Escrow Contract - Competition Details

Summary

abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() since it may lead to a hash collision. For example, abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456).

Vulnerability Details

There is 1 instance of this issue.

Impact

A hash collision would results in a incorrectly computed escrow address.

Tools Used

baudit: a custom static code analysis tool; manual review

Recommendations

To prevent hash collisions, use abi.encode() instead since it will pad the arguments to 32 bytes. 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. If all arguments are strings and/or bytes, bytes.concat() should be used instead.