hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 1 forks source link

Malicious users can front-run host users safe management actions and add those safes as root for wrong org #50

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0x97ab4711dcc1016ca5ded1ad1bcdb25311cc3bc94f515794e86e5db4a87a3a1e Severity: high

Description: Description\ When creating a rootSafe for organization, we only check whether the calling address is a valid root safe and if provided address is a safe:

 function createRootSafe(address newRootSafe, string calldata name)
        external
        IsSafe(newRootSafe)
        IsRootSafe(_msgSender())
        requiresAuth
        returns (uint256 safeId)
    {
        address caller = _msgSender();
        bytes32 org = getOrgHashBySafe(caller);
        uint256 newIndex = indexId;
        safeId = _createOrgOrRoot(name, caller, newRootSafe);
        // Setting level by default
        depthTreeLimit[org] = 8;

        emit Events.RootSafeCreated(org, newIndex, caller, newRootSafe, name);
    }

Note that a user can provide any newRootSafe, without guarantee if he is related to it. This is a big problem because once added in the system, newRootSafe cannot be added again from another organization. If this is weaponised, exploiter can DoS the composability of the palmera module by calling createRootSafe with address of other honest organization safes from his malicious organization. This will brick safe orchestration provided by palmera. This is so, because expoiter can prevent actors from calling addSafe and composing a tree. For example, malicious organisation can front-run addSafe by calling createRootSafe with address of the caller of addSafe. When honest party hit execution, it will revert on the following line:

        if (isSafeRegistered(caller)) {
            revert Errors.SafeAlreadyRegistered(caller);
        }

Attack Scenario\

  1. Alice creates malicious organization mal
  2. Bob creates root safe for his organisation.
  3. Bob has 4 safes for his employees in work and want to add them hierarchically with his safe as root.
  4. The manager-employee calls addSafe from his safe with superSafeId == {Bob root org safe}
  5. Alice sees the transaction and front-run it with createRootSafe with newRootSafe == manager safe.
  6. Manager safe is added to mal organization, which is useless.
  7. When manager transaction hit execution, it reverts, because his safe is already registered

Attachments

  1. Proof of Concept (PoC) File Will provide if needed

  2. Revised Code File (Optional) When calling createRootSafe, make sure newRootSafe has agreed to become root safe for the given organization. You can introduce another state var mapping(bytes[] => mapping(address => bool))) becomeRootForOrg and a method, which would be called from corresponding safes to switch flags