sherlock-audit / 2024-08-cork-protocol-judging

2 stars 2 forks source link

Parvez.eth - abi.encodePacked()` should not be used with dynamic types when passing the result to a hash function such as `keccak256()` #244

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

Parvez.eth

Invalid

abi.encodePacked()should not be used with dynamic types when passing the result to a hash function such askeccak256()`

Title

M-1 abi.encodePacked()should not be used with dynamic types when passing the result to a hash function such askeccak256()`

Summary

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.

Vulnerability Detail

4 Found Instances +- Found in src/contracts/core/assets/Asset.sol [Line: 75](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/assets/Asset.sol#L75) ```solidity ERC20(string(abi.encodePacked(prefix, "-", pairName)), string(abi.encodePacked(prefix, "-", pairName))) ``` - Found in src/contracts/core/assets/Asset.sol [Line: 76](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/assets/Asset.sol#L76) ```solidity ERC20Permit(string(abi.encodePacked(prefix, "-", pairName))) ``` - Found in src/contracts/core/assets/AssetFactory.sol [Line: 155](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/assets/AssetFactory.sol#L155) ```solidity string memory pairname = string(abi.encodePacked(Asset(ra).name(), "-", Asset(pa).name())); ``` - Found in src/contracts/core/assets/AssetFactory.sol [Line: 183](https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/assets/AssetFactory.sol#L183) ```solidity new Asset(LV_PREFIX, string(abi.encodePacked(Asset(ra).name(), "-", Asset(pa).name())), owner, 0, 0) ```
sherlock-admin3 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invalid. There is no dynamic values here.