DelegateToken's onERC721Received not work

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L83-L86

Vulnerability details

Impact

DelegateToken's onERC721Received not work

Proof of Concept

The onERC721Received function restricts the operator to only be address(this).

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L83-L86

    function onERC721Received(address operator, address, uint256, bytes calldata) external view returns (bytes4) {
        if (address(this) == operator) return IERC721Receiver.onERC721Received.selector;
        revert Errors.InvalidERC721TransferOperator();
    }

According to the EIP-721 specification, the operator is the caller of safeTransferFrom.

https://eips.ethereum.org/EIPS/eip-721#specification

/// @param _operator The address which called safeTransferFrom function

In the DelegateToken contract, there are only calls to safeTransferFrom for ERC1155, while for ERC721, it only calls transferFrom.

For example:

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L369

            IERC721(underlyingContract).transferFrom(address(this), msg.sender, erc721UnderlyingTokenId);

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L384

            IERC1155(underlyingContract).safeTransferFrom(address(this), msg.sender, erc11551UnderlyingTokenId, erc1155UnderlyingAmount, "");

https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenTransferHelpers.sol#L40-L42

    function pullERC721AfterCheck(address underlyingContract, uint256 underlyingTokenId) internal {
        IERC721(underlyingContract).transferFrom(msg.sender, address(this), underlyingTokenId);
    }

https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenTransferHelpers.sol#L70-L75

    function pullERC1155AfterCheck(Structs.Uint256 storage erc1155Pulled, uint256 pullAmount, address underlyingContract, uint256 underlyingTokenId) internal {
        IERC1155(underlyingContract).safeTransferFrom(msg.sender, address(this), underlyingTokenId, pullAmount, "");
        if (erc1155Pulled.flag == ERC1155_PULLED) {
            revert Errors.ERC1155NotPulled();
        }
    }

That is to say, even if users want to use DelegateToken as the underlyingContract, safeTransferFrom for ERC721 will not be called, thereby never triggering DelegateToken's onERC721Received. In other words, onERC721Received will never be activated.

Tools Used

Vscode

Recommended Mitigation Steps

In the contract, all transfers involving ERC721 do not use safeTransferFrom. I am not clear on the purpose of restricting address(this) == operator, so I don't know how to fix it.

Assessed type

ERC721

[GalloDaSballo]

/// @dev The ERC721 smart contract calls this function on the recipient
///  after a `transfer`. This function MAY throw to revert and reject the
///  transfer. Return of other than the magic value MUST result in the
///  transaction being reverted.

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid