Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Use `string.concat` instead of `abi.encodePacked` to concatenate strings

marcoroccon

Description:
The TokenDivider::divideNft function generates a new ERC20 token by creating a new instance of the ERC20ToGenerateNftFraccion contract, and passing two strings as the _name and _symbol parameters. These strings are defined as the concatenation of two strings, the name and symbol of the NFT, respectively with two strings literals. The concatenation is performed through the abi.encodePacked function, which is a deprecated method which could lead to hash collisions.

More on hash collision here:
https://docs.soliditylang.org/en/v0.8.13/abi-spec.html#non-standard-packed-mode

Tools Used:
Aderyn, manual review

Recommended Mitigation:
It is recommended to use the string.concat function instead. This function has been introduced since Solidity 0.8.12, improves code readibility, and is supported by the official documentation.

"Solidity does not have string manipulation functions, but there are third-party string libraries. You can also compare two strings by their keccak256-hash using keccak256(abi.encodePacked(s1)) == keccak256(abi.encodePacked(s2)) and concatenate two strings using string.concat(s1, s2)."

https://docs.soliditylang.org/en/v0.8.12/types.html#bytes-and-string-as-arrays

function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId) external {
​
if(nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); }
if(amount == 0) { revert TokenDivider__AmountCantBeZero(); }
​
ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(
- string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")),
- string(abi.encodePacked("F", ERC721(nftAddress).symbol())));
+ string.concat(ERC721(nftAddress).name(), "Fraccion"),
+ string.concat("F", ERC721(nftAddress).symbol());
...
}
Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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