hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 1 forks source link

Safe without children cannot become ROOT #47

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

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

Github username: -- Twitter username: SBSecurity_ Submission hash (on-chain): 0x6f1605ee79adeebcb5abdd118371fae970945d27bc037849c504d0c0c75bc4fc Severity: medium

Description: Description\ promoteRoot() is used to promote SAFEs to Root of an org. But only safes that have 0 children can become ROOTs which is unfair, because the actual ROOTs have children, why a SAFE with existing ones to cannot be promoted.

Attack Scenario\ If SAFE has 0 children and any of its ROOTs tries to promote him, promoteRoot() will fail, because of this check.

function promoteRoot(uint256 safeId)
        external
        IsRootSafe(_msgSender())
        SafeIdRegistered(safeId)
        requiresAuth
    {
        bytes32 org = getOrgBySafe(safeId);
        address caller = _msgSender();
        /// RootSafe usecase : Check if the safe is Member of the Tree of the caller (rootSafe)
        if (!isRootSafeOf(caller, safeId)) {
            revert Errors.NotAuthorizedUpdateNonChildrenSafe();
        }
        DataTypes.Safe storage newRootSafe = safes[org][safeId];
        /// Check if the safe is a Super Safe, and an Direct Children of thr Root Safe
        if (
            (newRootSafe.child.length <= 0) // @audit <-------------  
                || (!isSuperSafe(getSafeIdBySafe(org, caller), safeId))
        ) {
            revert Errors.NotAuthorizedUpdateNonSuperSafe();
        }
        /// Give Role RootSafe if not have it
        RolesAuthority _authority = RolesAuthority(rolesAuthority);
        _authority.setUserRole(
            newRootSafe.safe, uint8(DataTypes.Role.ROOT_SAFE), true
        );
        // Update Tier
        newRootSafe.tier = DataTypes.Tier.ROOT;
        // Update Root Safe
        newRootSafe.lead = address(0);
        newRootSafe.superSafe = 0;
        emit Events.RootSafePromoted(
            org, safeId, caller, newRootSafe.safe, newRootSafe.name
        );
    }

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) Allow even a safe with childs to become ROOT

alfredolopez80 commented 1 week ago

Non-issue is a expected behavior, because when you promote a SuperSafe to New rootSafe, the superSafe by definition have to be at least one children!!, so the if as you mention verify if the array of child safe is equal or less zero (not have children), and is not a superSafe and in this case can't promote to RootSafe, and revert the transactions with the error: NotAuthorizedUpdateNonSuperSafe()

so, is non-issue!!