search

sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

SanketKogekar - `abi.encodePacked` should stop to be used since there are conversions around to deprecate it in future versions of Solidity #310

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

SanketKogekar

medium

abi.encodePacked should stop to be used since there are conversions around to deprecate it in future versions of Solidity

Summary

In verifyPrices(), several dynamic arguments are passed to abi.encodePacked() which can cause certain issues in current/future versions as described below. It would be better for protocol to prefer abi.encode() just to be safe.

https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/libraries/LibMuon.sol#L50-L68

Vulnerability Detail

abi.encodePacked() will be removed according to Solidity guide https://github.com/ethereum/solidity/issues/11593 Also it will cause vulnerability in case of future update where you add dynamic types as arguments. Described in here https://docs.soliditylang.org/en/v0.8.19/abi-spec.html#non-standard-packed-mode

Impact

abi.encodePacked will behaviour unexpectedly in case of future Solidity update and should not be used. It will break the MerkleProof check if you add dynamic types in leaf argument in next protocol upgrade.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/libraries/LibMuon.sol#L50-L68

Tool used

Manual Review

Recommendation

Use abi.encode() or bytes.concat() instead of abi.encodePacked.