Incorrect `updateDepthTreeLimit` Check Can Lead to Unnecessary Denial of Service (DOS) for Root Safe

[hats-bug-reporter[bot]]

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0xd31a092e8db53d26eacddfb89af64985f814dc51553a21078ae5fb68e1537158 Severity: medium

Description: Description: The updateDepthTreeLimit function currently checks that the newLimit is not fewer than the existing depthTreeLimit[org], causing issues. For example, if the owner sets a new limit to 10 and then wants to update it to 9, the function prevents this update, causing unnecessary DOS for the root safe. The check should instead compare the newLimit to the default limit (8) and the current level of the organization.

Impact: The incorrect check leads to unnecessary DOS for root-safe operation, preventing legitimate updates to the depth tree limit.

Proof of Concept (PoC): The current updateDepthTreeLimit function:

function updateDepthTreeLimit(uint256 newLimit) external requiresAuth {
    bytes32 org = getOrgHashBySafe(_msgSender());
    if ((newLimit > maxDepthTreeLimit) || (newLimit <= depthTreeLimit[org])) {
        revert Errors.InvalidDepthTreeLimit();
    }
    depthTreeLimit[org] = newLimit;
    emit DepthTreeLimitUpdated(org, newLimit);
}

In the example scenario:

1. The owner sets a new limit to 10.
2. The owner wants to update the limit to 9.
3. The current check prevents this update, causing unnecessary DOS.

Mitigation: Update the updateDepthTreeLimit function to ensure that the newLimit is compared to the default limit (8) and the current level of the organization, rather than the existing depthTreeLimit[org].

if ((newLimit > maxDepthTreeLimit) || (newLimit < 8) || (newLimit <= current Level of org) )

[0xRizwan]

Centralized issue, Root safe/admins or functions called by Palmera admin are trusted and it is assumed that any action by such restricted functions would be correct. Imo, Non-issue. Would let sponsors decide.

[alfredolopez80]

Non-Issue, only RootSafe can update updateDepthTreeLimit and this value must be greater than before, and avoid any inconsistency with the entire tree that exists at the time of making the change.

Also @0xRizwan, I don't think it's a centralized issue, it's more to avoid inconsistency with the entire current tree, in case the limit is reached.