almurhasan - abi.encodePacked allows hash collision.

[sherlock-admin2]

almurhasan

medium

abi.encodePacked allows hash collision.

summary

From the solidity documentation: https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=collisions#non-standard-packed-mode > If you use keccak256(abi.encodePacked(a, b)) and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa. More specifically, abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c"). This issue exists in VestingEscrowFactory contract when deploying new VestingEscrow contract.

Vulnerability Detail

From the solidity documentation: https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=collisions#non-standard-packed-mode > If you use keccak256(abi.encodePacked(a, b)) and both a and b are dynamic types, it is easy to craft collisions in the hash value by moving parts of a into b and vice-versa. More specifically, abi.encodePacked("a", "bc") == abi.encodePacked("ab", "c"). This issue exists in VestingEscrowFactory contract when deploying new VestingEscrow contract.

Impact

two contract become of same address

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrowFactory.sol#L65

Tool used

Manual Review

Recommendation

Instead of using abi.encodePacked , use abi.encode.

[solimander]

Invalid. There are no dynamic inputs provided to abi.encodePacked.

[nevillehuang]

Invalid, there is no dynamic parameters highlighted during escrow deployment