sherlock-admin2
commented
1 year ago 3 comment(s) were left on this issue during the judging contest.
shogoki commented:
uint160 and address are both 160bits
0xyPhilic commented:
invalid because an address is 20 bytes which correspond to a uint160, so there is no possibility for same map of different tokens
Kral01 commented:
the likelihood is very low
feelereth
high
multiple token addresses could map to the same uint160 value, leading to collisions in the tokenIds
Summary
_encodeTokenId and _decodeTokenId rely on casting the token address to and from a uint160. This could lead to collisions if multiple token addresses map to the same uint160
Vulnerability Detail
The _encodeTokenId function takes a token address and casts it to a uint160: Link 1. This truncates the address to 160 bits. Since Solidity addresses are 160 bits, this works fine for encoding. However, the problem arises in _decodeTokenId: Link 2. Here, we are casting the uint tokenId back to a uint160 and then to an address. If two different token addresses happened to map to the same 160 bit integer, then this would return the same address for both!
_encodeTokenId and _decodeTokenId: Casting the address to uint160 means that any addresses that have the same lower 160 bits will map to the same tokenId. This could allow a malicious user to deposit one token but withdraw a different token that maps to the same id.
Impact
This allows a malicious user to deposit one token but withdraw a different token that maps to the same tokenId.
Code Snippet
https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/vault/HardVault.sol#L95-L97 https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/vault/HardVault.sol#L102-L104
Tool used
Manual Review
Recommendation
Use a more robust encoding scheme, like abi.encodePacked(), which incorporates the full address