GivingThanks

First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Invalid

Suboptimal Use of abi.encodePacked() for String Concatenation

Summary

The contract uses abi.encodePacked() for string concatenation in NFT metadata creation. While not critical in this implementation, better alternatives exist.

Vulnerability Details

string memory json = string(
abi.encodePacked(
'{"donor":"',
Strings.toHexString(uint160(donor), 20),
'","date":"',
Strings.toString(date),
'","amount":"',
Strings.toString(amount),
'"}'
)
);

Current implementation is safe because:

1. Used only for string concatenation
2. Different types are concatenated (address, uint256)
3. Not used with hash functions
4. Data has fixed JSON separators

Impact

Low - No direct security impact in current implementation

  • No risk of hash collisions

  • No dynamic data of same type being packed

  • Used only for metadata formatting

Tools Used

  • Manual code review

  • Solidity documentation analysis

Recommendations

Replace with bytes.concat() for better string handling:

string memory json = string(
bytes.concat(
bytes('{"donor":"'),
bytes(Strings.toHexString(uint160(donor), 20)),
bytes('","date":"'),
bytes(Strings.toString(date)),
bytes('","amount":"'),
bytes(Strings.toString(amount)),
bytes('"}')
)
);

This provides:

  • More explicit string handling

  • Better code readability

  • Following Solidity best practices for string operations

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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