thec00n - activateOperator does not update the OperatorUtilizationHeap

[sherlock-admin3]

thec00n

high

activateOperator does not update the OperatorUtilizationHeap

Summary

The activateOperator() function activates an inactive operator by increasing the activeOperatorCount by one and setting the operator status to active. This implementation is wrong as the OperatorUtilizationHeap does not include the inactive operator id after removing it.

Vulnerability Detail

The following example illustrates the issue (see also Code Snippet paragraph). The OperatorUtilizationHeap has 5 elements and the raw dump of the heap is as follows:

0: 0,0
1: 1,2
2: 5,10
3: 3,6,
4: 2,3
5:,4,8

Let's assume that the operator with index 3 is deactivated. The removal of an operator from the heap happens in setOperatorValidatorCap() by utilizationHeap.removeByID(operatorId) on line 303 in OperatorRegistryV1Admin. After the removal the heap only contains 4 elements. The raw dump of the heap shows that slot 3 has been overwritten with operator index 4 (see in the following code paragraph). So the heap now contains two elements for operator id 4. This is correct because removing the element also decreased the size of the heap by one. The activateOperator() increases the size of the heap by one without adding the element to the heap again. For the implementation, this means that the operator index with index 4 occurs twice on the heap, but the operator with index 3 that should be activated is missing.

0: 0,0
1: 1,2
2: 5,10
3: 4,8
4: 2,3
5: 4,8

Impact

Operator indexes on the heap must be unique, and they can not occur multiple times because this could cause severe accounting issues across the contract system.

Code Snippet

   function init() public pure returns ( OperatorUtilizationHeap.Data memory ){
        OperatorUtilizationHeap.Data memory heap;
        heap = OperatorUtilizationHeap.initialize(5);

        OperatorUtilizationHeap.Operator memory o1 = OperatorUtilizationHeap.Operator(1, 2);
        heap.insert(o1);

        OperatorUtilizationHeap.Operator memory o2 = OperatorUtilizationHeap.Operator(2, 3);
        heap.insert(o2);

        OperatorUtilizationHeap.Operator memory o3 = OperatorUtilizationHeap.Operator(3, 6);
        heap.insert(o3);

        OperatorUtilizationHeap.Operator memory o4 = OperatorUtilizationHeap.Operator(4, 8);
        heap.insert(o4);

        OperatorUtilizationHeap.Operator memory o5 = OperatorUtilizationHeap.Operator(5, 10);
        heap.insert(o5);

        return heap;
    }

    function removeByID() public pure returns ( uint, OperatorUtilizationHeap.Operator [] memory ){
        OperatorUtilizationHeap.Data memory heap = init();
        heap.removeByID(3);
        return (heap.count, heap.operators);
    }

Tool used

Manual Review

Recommendation

Use OperatorRegistryV1Admin.insert() function to add the inactive operator to the heap again.

Duplicate of #193