Lines of code

https://github.com/delegatexyz/delegate-registry/blob/6d1254de793ccc40134f9bec0b7cb3d9c3632bc1/src/DelegateRegistry.sol#L82-L99

Vulnerability details

Impact

Without explicitly validating its compliance with the ERC721 standard. This omission introduces a potential security vulnerability.

Proof of Concept

The delegateERC721 function in the contract allows users to delegate ERC721 tokens by interacting with an external contract specified by the contract_ address. However, this function lacks explicit validation to ensure that the contract_ address represents a valid ERC721 token contract. This omission could lead to potential vulnerabilities if an attacker deploys a malicious contract at contract_ that does not conform to the expected ERC721 standard but can still interact with this function.

Issue Explanation

In the delegateERC721 function, the contract interacts with an external ERC721 contract (contract_) without explicitly validating its compliance with the ERC721 standard. This omission introduces a potential security vulnerability.

Here's the relevant code snippet from the delegateERC721 function:

function delegateERC721(address to, address contract_, uint256 tokenId) external payable override returns (bytes32 hash) {
    // ...
    // No explicit validation for `contract_` as a valid ERC721 contract.
    // ...
}

In this function, the contract address is used without performing any validation to ensure that it represents a legitimate and compliant ERC721 token contract. This omission could lead to potential vulnerabilities if an attacker deploys a malicious contract at contract that does not conform to the expected ERC721 standard but can still interact with this function.

Potential Exploit Scenario

Consider a scenario where a user interacts with this contract's delegateERC721 function by providing an arbitrary contract address as contract_. The user deploys a contract that mimics the behavior of an ERC721 contract but is designed to be malicious. This malicious contract may not properly implement the ERC721 standard, but it can still receive and process calls from the delegateERC721 function.

In this scenario, the attacker could manipulate the malicious contract to perform unintended actions or withhold tokens, resulting in a loss of assets or disruption of the intended functionality of the smart contract.

Tools Used

Manual Review

Recommended Mitigation Steps

It is essential to perform explicit validation on the contract_ address to ensure that it conforms to the ERC721 standard before interacting with it. This can be done using the ERC721 interface or by verifying that the contract at contract_ implements the required ERC721 functions.

// Import the ERC721 interface
import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

function delegateERC721(address to, address contract_, uint256 tokenId) external payable override returns (bytes32 hash) {
    // Validate that `contract_` complies with ERC721
    require(IERC721(contract_).supportsInterface(0x80ac58cd), "Invalid ERC721 contract");

    // Rest of the function code
    // ...
}

In this example, we use the supportsInterface function from the ERC721 interface to validate that contract_ complies with the ERC721 standard. If it doesn't, the function will revert with an error message.

Assessed type

Invalid Validation

code-423n4 / 2023-09-delegate-findings

Lack of Validation for External ERC721 Contract in `delegateERC721` Function. #223