hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

Circles Protocol contracts
https://aboutcircles.com
GNU Affero General Public License v3.0
0 stars 0 forks source link

Missing ERC-165 and ERC-1155Receiver Interface Checks for Mint Policy and Treasury in (Custom) Group Registration #74

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x6a86c9ac2f892900e92a2411ba055caefc816ed444a5ccbd284fa3d4a54d1ee4 Severity: low

Description: Description\

Since the registerCustomGroup function allows a custom treasury address to be set and lacks a modification function to change it later, it's essential to ensure that the treasury address provided is correct and supports the ERC1155Receiver interface. However, the current code does not enforce this condition.

In the _registerGroup function, two important checks are missing that could lead to functional issues when interacting with the provided _mint (mint policy) and _treasury addresses:

  1. ERC-1155Receiver Check for Treasury: The contract does not verify whether the _treasury address supports the ERC1155Receiver interface, which is essential for properly handling ERC-1155 tokens. Without this check, tokens might be sent to a treasury that is not capable of receiving them, leading to asset loss or transaction reverts.

  2. ERC-165 Interface Check for Mint Policy: The contract does not check whether the _mint address (representing the mint policy) supports the required minting interface (using ERC-165). This could lead to errors if the mint policy contract does not implement the necessary functions to manage minting, potentially resulting in minting failures.

Attachments\

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

function _registerGroup(
    address _avatar,
    address _mint,
    address _treasury,
    string calldata _name,
    string calldata _symbol
) internal {
...
    // ERC-165 check to verify the mint policy supports the required interface
    require(
        IERC165(_mint).supportsInterface(type(IMintPolicy).interfaceId),
        "Mint policy must support required interface"
    );

    // ERC-1155Receiver check to ensure the treasury can receive ERC-1155 tokens
    require(
        IERC165(_treasury).supportsInterface(type(IERC1155Receiver).interfaceId),
        "Treasury must support ERC1155Receiver interface"
    );

Recommendation\

Add check as suggested above to ensure address parameters correctly filled to ensure treasury address can receive token correctly

benjaminbollen commented 1 month ago

Thank you for your report on the implementation of ERC-165 and ERC-1155Receiver interface checks for Mint Policy and Treasury in group registration. After review, we've determined this is not an issue.

Our current implementation adheres to the necessary ERC standards where applicable. The decision not to apply ERC-165 to policies or custom treasuries is deliberate, as it could potentially add a false sense of security at the cost of unneeded complexity.

We appreciate your attention to interface standards and security considerations. Thank you for your contribution to this security review.