Incorrect onERC1155Received call

[hats-bug-reporter[bot]]

Github username: @MehdiKarimi81 Twitter username: -- Submission hash (on-chain): 0x4b9e542ceaaf5bad3d98a378c86f23387ce75fa1867f516981bb5a643d3007f9 Severity: medium

Description:

Description

During the operateFlowMatrix operation, when the receiver of a stream is a group, the tokens are transferred to the treasury, and group tokens (circles) are minted for the group.

// 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);

However, the _callAcceptanceChecks function calls onERC1155Received on the receiver (which is the group in this case) with the data of the transferred tokens. The issue is that the tokens are not actually transferred to the receiver but to the treasury, yet onERC1155Received of the receiver is still called, even though the tokens have gone to the treasury.

             uint16 receiverCoordinate = _coordinates[3 * _streams[i].flowEdgeIds[0] + 2];
             address receiver = _flowVertices[receiverCoordinate];
            _acceptanceCheck(
                _flowVertices[_streams[i].sourceCoordinate], // from
                receiver, // to
                ids, // batch of Circles identifiers terminating in receiver
                amounts, // batch of amounts terminating in receiver
                _streams[i].data // user-provided data for stream
            );

Note: The group tokens minted for the receiver have a different token ID.

Attack Scenario

Alice calls operateFlowMatrix to stream circle tokens (with token IDs 1 and 2, each 10 in quantity) to a receiver that is a group. Since the receiver is a group, the hub transfers the tokens to the treasury and mints group tokens for the receiver. For example, let’s assume the group token has a token ID of 3, so 20 circle tokens (with token ID 3) are minted for the receiver.

Next, _callAcceptanceChecks calls onERC1155Received on the receiver with the data of the transferred tokens (in this case, token IDs 1 and 2, with 10 tokens each). Even though the tokens are transferred to the treasury, the receiver's onERC1155Received function is still called with the original token data.

[benjaminbollen]

Thank you for your report on the onERC1155Received call for groups in terminal path flows. After review, we've determined this is not an issue.

Your observation about the acceptance call being made with collateral tokens rather than the total amount of auto-minted group tokens is correct. This behavior is intentional, though we acknowledge the need for clearer documentation on this point.

We appreciate your careful examination of our ERC1155 implementation in the context of path transfers. Your input helps us improve our documentation and clarify system behaviors for developers. Thank you for your valuable contribution to this security review.

[benjaminbollen]

To provide more context on this behavior:

1. Our interpretation of the ERC1155 acceptance call in the context of path transfers aims to align with developer expectations with the concept of path transfers.

2. It's worth noting that we don't typically expect groups to be the end-receiver or terminal flow in a path. In fact, we anticipate that many group implementations may explicitly refuse the ERC1155 acceptance call to prevent direct token reception, as groups are primarily designed to define community rules.

3. However, we leave the final implementation decisions to developers building on top of our system. We will update our documentation to clearly explain this behavior and its implications for different use cases.

Thank you again for bringing this to our attention. Your insights help us improve the clarity and completeness of our documentation.