-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prefer abi.encode over abi.encodePacked #4
Labels
Comments
This was taken from the Uniswap repo so don't want to modify this. |
on second look, I'll keep this as best practice |
ghoul-sol
added
invalid
This doesn't seem right
and removed
invalid
This doesn't seem right
labels
Oct 12, 2021
This was referenced Jun 19, 2022
This was referenced Jun 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Handle
t11s
Vulnerability details
Impact
Use of abi.encodePacked in
PositionKey
is safe, but unnecessary and not recommended. abi.encodePacked can result in hash collisions when used with two dynamic arguments (string/bytes).PositionKey
does not use any dynamic types, but for maximum safety against future mistakes, using abi.encode is recommended.There is also discussion of removing abi.encodePacked from future versions of Solidity (ethereum/solidity#11593), so using abi.encode now will ensure compatibility in the future.
Proof of Concept
https://github.com/code-423n4/2021-09-wildcredit/blob/edfc97a8b75bb290a7f7f61cda62d586ed42c73e/contracts/external/PositionKey.sol#L11
Recommended Mitigation Steps
Use abi.encode instead.
The text was updated successfully, but these errors were encountered: