-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using knowledge from previous blocks can result in Backup Operator manipulation #169
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate
This issue or pull request already exists
responded
The Holograph team has reviewed and responded
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Comments
code423n4
added
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
labels
Oct 24, 2022
This was referenced Oct 30, 2022
Closed
Closed
Closed
referenced the wrong issue, ignore the above dupes |
This was referenced Oct 30, 2022
gzeoneth
added
the
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
label
Oct 30, 2022
gzeoneth
added
the
primary issue
Highest quality submission among a set of duplicates
label
Oct 30, 2022
gzeoneth
added
duplicate
This issue or pull request already exists
and removed
primary issue
Highest quality submission among a set of duplicates
labels
Nov 19, 2022
gzeoneth
added
3 (High Risk)
Assets can be stolen/lost/compromised directly
and removed
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
labels
Nov 21, 2022
I reported as low in my QA report L-06, please take a look |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate
This issue or pull request already exists
responded
The Holograph team has reviewed and responded
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
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.
The text was updated successfully, but these errors were encountered: