Summary

abi.encodePacked() with dynamic types (like strings) can lead to hash collisions if the result is hashed. Although this specific code doesn't hash the result, using abi.encodePacked with dynamic types is discouraged as per best practices. The recommendation is to use bytes.concat() when dealing with bytes/strings to ensure safe concatenation without unexpected collisions.

Vulnerability Details

Technical Explanation:

1. abi.encodePacked() Behavior:

   • abi.encodePacked() tightly packs input data without padding. For dynamic types (e.g., string, bytes), this can lead to ambiguous concatenation. For example:

     • abi.encodePacked("Hello", "World") and abi.encodePacked("Hell", "oWorld") both produce the same byte sequence (0x48656c6c6f576f726c64).

   • If these results are used in hashing (e.g., for generating identifiers, signatures, or verification), collisions can occur, leading to security vulnerabilities (e.g., signature replay attacks, identifier spoofing).

2. Contract-Specific Risk:

   • In divideNft, the ERC20 token name and symbol are derived from the NFT’s name and symbol. For example:

     solidity

     Copy

     string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion"))

   • If two different NFTs have names that produce the same packed byte sequence when concatenated with "Fraccion", their ERC20 tokens would share the same name and symbol. This could mislead users or enable fraudulent token impersonation.

Impact

If two different inputs produce the same packed output (e.g., encodePacked("ab", "c") vs. encodePacked("a", "bc")), their concatenated results would be identical. This could lead to unintended behavior if these results are used in critical operations.

Tools Used

Aderyn

Recommendations

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.
If all arguments are strings and or bytes, bytes.concat() should be used instead.