[DRO-01M] Potential Lock of EIP-721 Assets

[chasebrownn]

DRO-01M: Potential Lock of EIP-721 Assets

Type	Severity	Location
Logical Fault		Delegator.sol:L101, L122-L124

Description:

The Delegator::depositDelegatorToken function will inefficiently transfer the EIP-721 asset inward as it will use an IERC721::safeTransferFrom variant that necessitates the presence of the Delegator::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 Delegator contract even if an IERC721::safeTransferFrom operation is utilized.

Example:

/**
 * @notice This method is used to deposit a delegated veRWA NFT into this contract.
 * @dev There should only be 1 NFT deposited during the lifespan of this delegator.
 * @param _tokenId Token identifier of veRWA token.
 */
function depositDelegatorToken(uint256 _tokenId) external {     
    require(msg.sender == delegateFactory, "Delegator: Not authorized");

    delegatedToken = _tokenId;   

    veRWA.safeTransferFrom(msg.sender, address(this), _tokenId);
    Votes(address(veRWA)).delegate(delegatee);

    emit TokenDelegated(_tokenId, delegatee);
}

/**
 * @notice This method is used to transfer the `delegatedToken` back to the `creator`.
 */
function withdrawDelegatedToken() external {
    require(msg.sender == delegateFactory || msg.sender == owner(), "Delegator: Not authorized");

    Votes(address(veRWA)).delegate(address(this));
    veRWA.safeTransferFrom(address(this), creator, delegatedToken);

    emit DelegationWithdrawn(delegatedToken, delegatee);
}

/**
 * @notice Allows address(this) to receive ERC721 tokens.
 */
function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
    return IERC721Receiver.onERC721Received.selector;
}

Recommendation:

We advise a direct IERC721::transferFrom operation to be utilized, removing the Delegator::onERC721Received function requirement and thus optimizing the code's gas while preventing funds from being accidentally locked in the contract.

[chasebrownn]

Resolved