Reducing randomness can force open season

[code423n4]

Lines of code

https://github.com/holographxyz/holograph-protocol/blob/c4_audit/contracts/HolographOperator.sol#L849

Vulnerability details

Impact

User can call bondUtilityToken function with large pod value which ensure pushing [address(0)] from _operatorPods.length till pod.

This becomes a problem _operatorPods.length increases dramatically causing randomess to reduce significantely in crossChainMessage. Reason being random % _operatorPods.length will mostly result in unused pod with address(0) as only operator, causing open season where anyone can execute the job

Proof of Concept

1. User A calls the bondUtilityToken function with pod say 100

  function bondUtilityToken(
    address operator,
    uint256 amount,
    uint256 pod
  ) external {
...
for (uint256 i = _operatorPods.length; i <= pod; i++) {
          /**
           * @dev add zero address into pod to mitigate empty pod issues
           */
          _operatorPods.push([address(0)]);
        }
...
}

2. Lets say _operatorPods initially had 2 elements so after calling this function and pod as 100 this will push address(0) 98 times making the _operatorPods length as 100

3. This becomes a problem while calling crossChainMessage function

function crossChainMessage(bytes calldata bridgeInRequestPayload) external payable {
...
uint256 pod = random % _operatorPods.length;
uint256 podSize = _operatorPods[pod].length;
 uint256 operatorIndex = random % podSize;
...
}

4. Assume random is selected as 10, which makes pod as 10 and _operatorPods[pod].length is 1 (while initializing one dummy operator with address 0 is added). This makes operatorIndex as 0 (random%1=0)

5. Now 0 operatorIndex means open season and anyone can execute the job

/**
       * @dev If operator index is 0, then it's open season! Anyone can execute this job. First come first serve
       *      pop operator to ensure that they cannot be selected for any other job until this one completes
       *      decrease pod size to accomodate popped operator
       */

Recommended Mitigation Steps

Do not allow user to specify a very large value for pods

[gzeoneth]

As noted in #256, this seems to be bounded since the cost of joining higher pod increase exponentially

[alexanderattar]

Design decision. Operator that bonds massive amounts of tokens to a higher than next pod is purposefully punished by decreasing their probability of exclusive job rights. Forces a gradual bonding curve

[gzeoneth]

#432 as primary