code-423n4 / 2022-10-holograph-findings

1 stars 0 forks source link

Weak randomness #474

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/src/HolographOperator.sol#L400

Vulnerability details

Vulnerability details

Description

In the function crossChainMessage of HolographOperator contract there is the following logic implemented for the calculation of the random value:

/**
 * @dev use job hash, job nonce, block number, and block timestamp for generating a random number
 */
uint256 random = uint256(keccak256(abi.encodePacked(jobHash, _jobNonce(), block.number, block.timestamp)));

Below in this function, such a value is used in the following way:

/**
 * @dev divide by total number of pods, use modulus/remainder
 */
uint256 pod = random % _operatorPods.length;
/**
 * @dev identify the total number of available operators in pod
 */
uint256 podSize = _operatorPods[pod].length;
/**
 * @dev select a primary operator
 */
uint256 operatorIndex = random % podSize;
/**
 * @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
 */
_operatorTempStorage[_operatorTempStorageCounter] = _operatorPods[pod][operatorIndex];
_popOperator(pod, operatorIndex);
if (podSize > 1) {
  podSize--;
}
_operatorJobs[jobHash] = uint256(
  ((pod + 1) << 248) |
    (uint256(_operatorTempStorageCounter) << 216) |
    (block.number << 176) |
    (_randomBlockHash(random, podSize, 1) << 160) |
    (_randomBlockHash(random, podSize, 2) << 144) |
    (_randomBlockHash(random, podSize, 3) << 128) |
    (_randomBlockHash(random, podSize, 4) << 112) |
    (_randomBlockHash(random, podSize, 5) << 96) |
    (block.timestamp << 16) |
    0
); // 80 next available bit position && so far 176 bits used with only 128 left

Using flashbots an attacker can predefine block parameters such as block.number and block.timestamp, _jobNonce value. jobHash value is obviously known for the caller. So there exists a way to predict the "random" value and mine job parameters in such a way that pod and operatorIndex values will be equal to the needed for the caller.

Recommended Mitigation Steps

Use Chainlink's (or another trusted offchain information provider's) random value generation.

gzeoneth commented 1 year ago

Duplicate of #27

ACC01ADE commented 1 year ago

Pseudo randomness is intentionally being used and is a design choice. The ability to manipulate the transaction with flashbots (or other MEW tech) is not disputed, but the cost-to-benefit ratio is not valid and justifiable enough to make this an actual issue. Thus disputing the severity of the issue to being low severity at best.