hats-finance / Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be

Palmera hierarchical module
0 stars 1 forks source link

Updating depthtreelimit is ineffective #4

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

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

Github username: @whoismxuse Twitter username: mxuse Submission hash (on-chain): 0xdc8f13fc9f47309f2617214ff27dce956df8a8b85156ef879c55cbaa5ab87f90 Severity: medium

Description:

Description

Inside PalmeraModule.sol the following function allows for a new updated depth of the treelimit:


    /// @dev Method to update Depth Tree Limit
    /// @param newLimit new Depth Tree Limit
    function updateDepthTreeLimit(uint256 newLimit)
        external
        IsRootSafe(_msgSender())
        requiresAuth
    {
        address caller = _msgSender();
        bytes32 org = getOrgHashBySafe(caller);
        uint256 rootSafe = getSafeIdBySafe(org, caller);
        if ((newLimit > maxDepthTreeLimit) || (newLimit <= depthTreeLimit[org]))
        {
            revert Errors.InvalidLimit();
        }
        emit Events.NewLimitLevel(
            org, rootSafe, caller, depthTreeLimit[org], newLimit
        );
->        depthTreeLimit[org] = newLimit;
    }

As you can see at the end of the function the depthTreeLimit[org] gets updated to the newLimit.

However due to the hardcoding of the depthTreeLimit inside several places of the contract, it will still remain 8:

    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);
    }

For example inside createRootSafe, the depth is hardcoded to 8 so any changes to the depth will not be reflected inside this function even though it should.

Tools Used

Manual Review

Recommendation

remove the hardcoded depthTreeLimit[name] = 8

0xRizwan commented 1 week ago

Non-issue, When the root safe is created, the default value is set to 8 for depthTreeLimit but it can be updated by root safe. Function is implemented correctly.

alfredolopez80 commented 1 week ago

Non-issue like @0xRizwan mention