Unbonded `orgHash` Could Result in Denial of Service (DOS)

[hats-bug-reporter[bot]]

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0x03beff97c48e17388e7e20e197e82bf6aef4a2695474d532517022731f6536a9 Severity: high

Description: Description: Unbonded orgHash could result in a denial of service (DOS) in several functions, potentially leading to serious issues. The affected functions are:

• getOrgBySafe
• removeOrg
• getOrgHashBySafe

Impact: Denial of service in core functions, potentially affecting the integrity and usability of the contract. A DOS attack in the removeOrg function can also cause issues when attempting to remove an organization from the list of organization hashes. (due to this I set it as high)

Scenario: An attacker can exploit unbonded orgHash to cause these functions to fail, preventing legitimate users from interacting with the contract. For example:

1. An attacker creates lots of orgs..
2. The contract becomes unable to process legitimate orgHash values due to the presence of unbonded values, leading to DOS in the mentioned functions.

[alfredolopez80]

@0xmahdirostami and @0xRizwan maybe need to discuss this:

1. removeOrg is a private function, can't access directly!! only thought removeWholeTree() or disconnectSafe(safeId)
2. second this is like the DNS in the 90 how we now is the owner is the legitimate owner of the orgHash name? we need it something like KYC before to create a OnChain Organization and this is complete centralized? i guess need to analyze this more deep

[0xmahdirostami]

@alfredolopez80 thank you.

1. i just point to a direct function removeOrg. Yes, removeOrg is private and used in disconnectSafe and removeWholeTree, so if removeOrg revert due to unbounded orgHash.length, it will cause DOS in disconnectSafe and removeWholeTree.
2. everyone with safe wallet could call registerOrg, and increase the length of orgHashes, if you want to prevent it you could create a whitelist mechanism.

[0xRizwan]

@alfredolopez80 yes, as said by Mahdi whitelisting mechanism can be considered to prevent this issue. The possibility of occurence of such issues in real world is quite low unless the motive is get some monetary gains and monetary gains can not be acheived even the attacker has to waste his money on paying gas fees so i think, a low severity can be considered for this issue.

[0xmahdirostami]

@alfredolopez80 yes, as said by Mahdi whitelisting mechanism can be considered to prevent this issue. The possibility of occurence of such issues in real world is quite low unless the motive is get some monetary gains and monetary gains can not be acheived even the attacker has to waste his money on paying gas fees so i think, a low severity can be considered for this issue.

I don't think the impact of this issue (adding a contract in an insolvency state and DOSing important functions) could be low.

[alfredolopez80]

i agree @0xmahdirostami and @0xRizwan

[0xmahdirostami]

i agree @0xmahdirostami and @0xRizwan

Ser, the issue isn't low.

[NicolaMirchev]

Hey, @alfredolopez80

I believe the issue is pretty serious, because getOrgBySafe is used in almost (maybe all) other functions in Palmera Module. This means that the whole contract may be bricked and out of service. Having in mind that anybody can indefinitely increase the size of the array is a big red flag. If someone wants, he can 100% DoS the whole contract. Also, malicious actor can make interaction with the module super expensive, especially for Mainnet, which is also a bad thing.

[0xRizwan]

Anyone can register an org which brings the possibility of this issue to some extent. Further, registerOrg only check the caller is saf e and i think that check can also be bypassed. I believe, this issue can be considered as Medium severity since it can create a DOS. Would leave upto @alfredolopez80 on final severity of this issue.

[alfredolopez80]

this is not a duplicate issue #3 ?

[0xmahdirostami]

The issue #3 is about not letting a real user call registerOrg, which can cause gas grief and Dos.

this issue isn't just about registerOrg, it's about getOrgBySafe as well, that is used in almost (maybe all) other functions in Palmera Module. This means that the whole contract may be bricked and out of service. If someone wants, he can 100% DoS the whole contract.