The DelegateFactory::deployDelegator function will inefficiently transfer the EIP-721 asset inward as it will use an IERC721::safeTransferFrom variant that necessitates the presence of the DelegateFactory::onERC721Received function, thereby allowing any EIP-721 asset to be locked even if a safe transfer is performed.
Impact:
It is presently possible to lock any EIP-721 asset to the DelegateFactory contract even if an IERC721::safeTransferFrom operation is utilized.
We advise a direct IERC721::transferFrom operation to be utilized, removing the DelegateFactory::onERC721Received function requirement and thus optimizing the code's gas while preventing funds from being accidentally locked in the contract.
chasebrownn
commented
6 months ago
DFY-01M: Potential Lock of EIP-721 Assets
Description:
The
DelegateFactory::deployDelegatorfunction will inefficiently transfer the EIP-721 asset inward as it will use anIERC721::safeTransferFromvariant that necessitates the presence of theDelegateFactory::onERC721Receivedfunction, thereby allowing any EIP-721 asset to be locked even if a safe transfer is performed.Impact:
It is presently possible to lock any EIP-721 asset to the
DelegateFactorycontract even if anIERC721::safeTransferFromoperation is utilized.Example:
Recommendation:
We advise a direct
IERC721::transferFromoperation to be utilized, removing theDelegateFactory::onERC721Receivedfunction requirement and thus optimizing the code's gas while preventing funds from being accidentally locked in the contract.