hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

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

Lack of Organizational Minting Capability #21

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): 0xd76b9360f866bf4b47e5c5d7764443140600cb880771c92720fb785d2dc3d66e Severity: low

Description: Description\ The current implementation of the smart contract does not allow organizations or groups to mint tokens, which contradicts the intended functionality outlined in the documentation. Specifically, the groupMint() function enforces that the _group parameter must pass a validation check (isGroup(_group)), but the function ultimately restricts the minting process if the group fails the required conditions.

In the contract, personal minting is allowed through the personalMint() function, which checks if the msg.sender is a human by calling isHuman(msg.sender). However, there is no corresponding function or condition that directly enables organizations to mint tokens independently, even though the documentation references a "Consented Flow" feature intended to support group or organizational control over token usage.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept src\hub\Hub.sol

    function groupMint(
    address _group,
    address[] calldata _collateralAvatars,
    uint256[] calldata _amounts,
    bytes calldata _data
    ) external {
    uint256[] memory collateral = new uint256[](_collateralAvatars.length);
    for (uint256 i = 0; i < _collateralAvatars.length; i++) {
        collateral[i] = toTokenId(_collateralAvatars[i]);
    }
    _groupMint(msg.sender, msg.sender, _group, collateral, _amounts, _data, true);
    }
    function _groupMint(
    address _sender,
    address _receiver,
    address _group,
    uint256[] memory _collateral,
    uint256[] memory _amounts,
    bytes memory _data,
    bool _explicitCall
    ) internal {
    if (_collateral.length != _amounts.length) {
        // Collateral and amount arrays must have equal length.
        revert CirclesArraysLengthMismatch(_collateral.length, _amounts.length, 1);
    }
    if (_collateral.length == 0) {
        // At least one collateral must be provided.
        revert CirclesArrayMustNotBeEmpty(0);
    }
    if (!isGroup(_group)) {
        // Group is not registered as an avatar.
        revert CirclesHubGroupIsNotRegistered(_group, 0);
    }
    
    // note: we don't need to check whether collateral circle ids are registered,
    // because only for registered collateral do non-zero balances exist to transfer,
    // so it suffices to check that all amounts are non-zero during summing.
    uint256 sumAmounts = 0;
    for (uint256 i = 0; i < _amounts.length; i++) {
        address collateralAvatar = _validateAddressFromId(_collateral[i], 1);
    
        // check the group trusts the collateral
        // and if the sender has opted into consented flow, the sender must also trust the the group
        bool isValidCollateral =
            _explicitCall ? isTrusted(_group, collateralAvatar) : isPermittedFlow(_sender, _group, collateralAvatar);
    
        if (!isValidCollateral) {
            // Group does not trust collateral, or flow edge is not permitted
            revert CirclesHubFlowEdgeIsNotPermitted(_group, _collateral[i], 0);
        }
    
        if (_amounts[i] == 0) {
            // Non-zero collateral must be provided.
            revert CirclesAmountMustNotBeZero(0);
        }
        sumAmounts += _amounts[i];
    }
    
    // Rely on the mint policy to determine whether the collateral is valid for minting
    if (!IMintPolicy(mintPolicies[_group]).beforeMintPolicy(_sender, _group, _collateral, _amounts, _data)) {
        // Mint policy rejected mint.
        revert CirclesHubGroupMintPolicyRejectedMint(_sender, _group, _collateral, _amounts, _data, 0);
    }
    
    // abi encode the group address into the data to send onwards to the treasury
    bytes memory metadataGroup = abi.encode(GroupMintMetadata({group: _group}));
    bytes memory dataWithGroup = abi.encode(
        Metadata({metadataType: METADATATYPE_GROUPMINT, metadata: metadataGroup, erc1155UserData: _data})
    );
    
    // note: treasury.on1155Received must implement and unpack the GroupMintMetadata to know the group
    safeBatchTransferFrom(_sender, treasuries[_group], _collateral, _amounts, dataWithGroup);
    
    // mint group Circles to the receiver and send the original _data onwards
    _mintAndUpdateTotalSupply(_receiver, toTokenId(_group), sumAmounts, _data);
    }
    function personalMint() external {
    if (!isHuman(msg.sender)) {
        // Only avatars registered as human can call personal mint.
        revert CirclesHubMustBeHuman(msg.sender, 1);
    }
    // check if v1 Circles is known to be stopped and update status
    _checkHumanV1CirclesStatus(msg.sender);
    
    // claim issuance if any is available
    _claimIssuance(msg.sender);
    }
  2. Revised Code To address this issue, it is recommended to introduce a clear mechanism that enables organizations to mint tokens.
JordanRaphael commented 1 month ago

An short edit in the description:

The current implementation of the smart contract does not allow organizations to mint tokens, which contradicts the intended functionality outlined in the documentation. Specifically, the groupMint() function enforces that the _group parameter must pass a validation check (isGroup(_group)). In the contract, personal minting is allowed through the personalMint() function, which checks if the msg.sender is a human by calling isHuman(msg.sender). However, there is no corresponding function or condition that directly enables organizations to mint tokens independently, even though the documentation references a "Consented Flow" feature intended to support organization control over Circle token usage.

benjaminbollen commented 1 month ago

Thank you for your report on the lack of organizational minting capability. After review, we've determined that this is not an issue, but rather a fundamental design choice in our system. Organizations are intentionally defined by their inability to mint their own tokens. This is a key feature that distinguishes organizations from other entities in our ecosystem. We appreciate your attention to the functionalities of different entity types in our system. While this limitation is by design, your report helps ensure our system's features and constraints are clearly understood. Thank you for contributing to the ongoing discussion about our platform's architecture.