Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L132-L149

Vulnerability details

Impact

Operator may be removed without checking whether are there fund locked in that operator. Locked fund may not be able to withdraw unless operator is being added back.

Proof of Concept

    /// @inheritdoc INestedFactory
    function removeOperator(bytes32 operator) external override onlyOwner {
        bytes32[] storage operatorsCache = operators;
        uint256 operatorsLength = operatorsCache.length;
        for (uint256 i = 0; i < operatorsLength; i++) {
            if (operatorsCache[i] == operator) {
                operatorsCache[i] = operators[operatorsLength - 1];
                operatorsCache.pop();
                if (operatorCache[operator].implementation != address(0)) {
                    delete operatorCache[operator]; // remove from cache
                }
                rebuildCache();
                emit OperatorRemoved(operator);
                return;
            }
        }
        revert("NF: NON_EXISTENT_OPERATOR");
    }

No checking of locked fund in the operator.

Tools Used

Manual

Recommended Mitigation Steps

Add some checking function to operator interface
Call that function to check for removability of that operator before remove it

code-423n4 / 2022-06-nested-findings

Operator may be removed without checking whether are there fund locked in that operator. #80