hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

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

Incomplete Interface Inheritance in the `InflationaryCircles` Contract #87

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

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

Description: Description\ The InflationaryCircles contract is designed to handle inflationary balances and interact with ERC20 and ERC1155 standards. However, during the review, an issue was identified regarding the inheritance and implementation of the IHubV2 interface. The contract is supposed to follow the specifications of IHubV2, which governs the main logic for Circles protocol interactions, including managing multi-token ERC1155 contracts, trust relations, and demurraged token balances.

Upon inspection, the contract declares IHubV1 as a state variable:

IHubV1 public hub;

Which is fine to import migration/IHub for it, but the other functions used in the contract should be following IHubV2 interace i.e isHuman, isGroup & isOrganization etc.

But it does not inherit directly from the IHubV2 interface. The contract should be directly inheriting from IHubV2 to ensure that all necessary functions and properties from the interface are implemented and enforced. This missing inheritance can lead to unintended behavior and security issues when interacting with the Circles protocol.

Impact\ Without inheriting the IHubV2 interface, the contract is not explicitly required to implement essential methods that are defined in the interface. This could lead to critical functionality being omitted or not adhering to the standard specified in the Circles protocol. Lack of direct inheritance opens the contract to potential vulnerabilities, as developers or users may assume that the contract implements all functions and standards defined by IHubV2, when in reality it does not. This can be exploited by malicious actors, especially in cases where critical trust or token-handling functions are omitted or incorrectly implemented.

Recommendation\ To resolve this issue, the InflationaryCircles contract should explicitly inherit from IHubV2. This will ensure that the contract is bound to implement all the necessary methods and properties, thereby aligning with the Circles protocol and preventing security gaps

benjaminbollen commented 2 months ago

Thank you for your report regarding the interface inheritance in the InflationaryCircles contract. After review, we’ve determined this is not an issue.

The contract correctly references the hub (v2) as "IHubV2" rather than "IHubV1," see https://github.com/aboutcircles/circles-contracts-v2/blob/rc-v0.3.6-alpha/src/lift/InflationaryCircles.sol#L18.

And there is no need for the ERC20 to implement the IHubV2 interface, because it is not the hub.

We appreciate your effort, but no further action is required on this matter.