Closed code423n4 closed 1 year ago
referenced the wrong issue, ignore the above dupes
also see #167 #168 from the same warden with some high quality analysis
Really good issue here. Really appreciate the #167 and #168 analysis as well.
I reported as low in my QA report L-06, please take a look
Lines of code
https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L1185-L1193 https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/HolographOperator.sol#L522-L533
Vulnerability details
Using knowledge from previous blocks can result in Backup Operator manipulation
Impact
In contracts/HolographOperator.sol#crossChainMessage, after an Operator is selected (see related issue
H001
), the remainder job details are filled (L522-L533). This includes a call to _randomBlockHash, providing therandom
number used to select a particular pod, the pod size (podSize
), and a sequential number (n
) for each of the five Backup Operators.The index for each Operator is calculated as
(random + uint256(blockhash(block.number - n))) % podSize
. This approach has three problems:random
as a criterion to select both the pod as the Backup Operators(A+B) % C == ( (A%C) + (B%C) ) % C
.In some instances, an attacker may correctly predict in which indexes a Backup Operator will be chosen. Regarding 1., every time a pod is selected, all
random
numbers will have a predetermined characteristic (modulus result equals X). When combining 1. with 3., we have a fixed set of possible indexes. An attacker can then monitor a particular pod size and, by easy access to the second criterion (2. previous n blockhash), can automate and influence which indexes will be selected as Operator Backups.Impacts for this issue include:
Proof of Concept
For the sake of simplicity, consider the following scenario:
At this stage, an attacker Mallory has joined pod0, with an Operator (
B1
) in index2
. She keeps an eye on its size, because she knows that whenever arandom
(% number_of_pods = 0
) fall topod0
, if thepod0_size
is close to12
, there's an opportunity to influence the chosen index due to the above-mentioned point 3. So, without manipulation:After checking for potential benefits of the previous BX, Mallory deploys two additional Operators,
pod0_size
increases to12
and whichever random falls topod0
won't have any effect in choosing the next backups, sincerandom % 12 == 0
:Mallory needs to keep her place in the pod to execute the job at the right time (she'll be the first backup), which may not be as harder as it seems due to the pop mechanism (swapping with the last spot), she has good odds to keep her spot or, as a safeguard, ensure that the last position is hers in case she is selected for another job.
Although this PoC shows a fairly easy way how an attacker can take advantage of this design flaw, extreme scenarios may need some additional requirements (in a mev abundant world, if there is money to be made, it is likely to happen):
Tools Used
Manual
Recommended Mitigation Steps
Has stated in
M001-Biased distribution
, use two random numbers for pod, Operator selection and now backup selection. Ideally, an independent source for randomness should be used, but following the assumption that the one used in L499 is safe enough, therandom
may have enough entropy to serve the additional requirements (use right shift to and divide into 6 blocks).Additionally, since randomness in the blockchain is always tricky to achieve without an oracle provider, consider adding additional controls (e.g. waiting times before joining each pod) to increase the difficulty of manipulating the protocol.
And finally, in this particular case, removing the swapping mechanism (moving the last index to the chosen operator's current index) for another mechanism (e.g. shuffling) could also increase the difficulty of manipulating a particular pod.