Multiple ERC721 pools can be created for the same subset of TokenIDs
Summary
The deployPool function of ERC721Factory uses kecchak256 hash of a uint array input by the user, which can be manipulated using the order of elements
Vulnerability Detail
The array is a user parameter, so the order of its elements depends on the user. While the docs show the intention to create a single pool for a particular subset of NFT tokenIDs as it checks if a pool is already deployed then do not deploy it again, the subsethash can be different for the same subset of IDs because of the order in which they are entered in the tokenIDs array.
This could lead to multiple pools with the same list of tokenIDs of an NFT contract, possibly with different interest rates etc., while this is unintended behavior.
A possible mitigation is to sort the elements of the tokenIDs array in either ascending/descending order for all pool creation transactions. Then take the kecchak256 and check the existence of such a pool.
Chinmay
medium
Multiple ERC721 pools can be created for the same subset of TokenIDs
Summary
The deployPool function of ERC721Factory uses kecchak256 hash of a uint array input by the user, which can be manipulated using the order of elements
Vulnerability Detail
The array is a user parameter, so the order of its elements depends on the user. While the docs show the intention to create a single pool for a particular subset of NFT tokenIDs as it checks if a pool is already deployed then do not deploy it again, the subsethash can be different for the same subset of IDs because of the order in which they are entered in the tokenIDs array.
This could lead to multiple pools with the same list of tokenIDs of an NFT contract, possibly with different interest rates etc., while this is unintended behavior.
Impact
Code Snippet
https://github.com/sherlock-audit/2023-01-ajna/blob/main/contracts/src/ERC721PoolFactory.sol#L108
Tool used
Manual Review
Recommendation
A possible mitigation is to sort the elements of the tokenIDs array in either ascending/descending order for all pool creation transactions. Then take the kecchak256 and check the existence of such a pool.
Duplicate of #73