hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 1 forks source link

Safes in an org can be stuffed up to the TreeDepthLimitReached by anyone #46

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

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

Github username: @@giorgiodalla Twitter username: 0xAuditism Submission hash (on-chain): 0xfcf16772bfec6d7263598bb0e252bb6ab328ce401ed0337dce0b7154a535dd6c Severity: medium

Description: Description\ The addSafe() function allows the caller (a safe) to add it to an organisation. Every organisation has a threshold of safes it can hold, default is set to 8. Given this, anyone can stuff any given org to the TreeDepthLimit with Safes they don't want. Furthermore since this project is meant to be deployed on mainnet, an attacker could simply backrun the deletion of a Safe, by adding a Safe before the oragnisation manages to.

Attack Scenario\

Bob sees that org BANYA has a threshold of 5 safes and currently holds 3 Safes.

Bob decides to stuff BANYA org with safes. Now BANYA has 5 Safes, 2 they don't want. They want to clear more space but Bob continues to add a Safe whenever a Safe is deleted.

Attachments

  1. Proof of Concept (PoC) File
function addSafe(uint256 superSafeId, string memory name)
        external
        SafeIdRegistered(superSafeId)
        IsSafe(_msgSender())
        returns (uint256 safeId)
    {

Here above we can see that any valid Safe can be added to any registered org. Which happens to be problematic if undesirable safes are added to the org.

  1. Revised Code File (Optional)

Consider enabling a whitelisting method for the Safes that can be added to an org.

alfredolopez80 commented 6 days ago

Non-Issue, @0xRizwan we have a stress-test in we can create a Onchain-Organization with 8100 Safes with any run out gas problem, additional we limit the maxdeepTreelimit to 50, to avoid this situation!!

Additional de default value if more than enough for any org!!, beside the rootSafe can disconnectSafe any safe or remoteWholeTree is if necessary!!

i invite to check this stress test in the test folder, was writed in foundry