Skip to content
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

Operator incentivization is not implemented properly, making it unlikely new operators will join and destabilization the network. #449

Closed
code423n4 opened this issue Oct 25, 2022 · 4 comments
Labels
bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate This issue or pull request already exists QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 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
Copy link
Contributor

Lines of code

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

Vulnerability details

Description

Operators can choose which pod level to belong to, with each one having a different bond amount. The docs state :
"The probability of job selection is based on a specific pod being joined, and the number of Operators bonded in the specific Pod. "

This makes sense. Because the operator's only revenue is through fees collection for each operation, which is the same for every pod, the only incentive to go for a higher pod is higher frequency of job execution.

However, the crossChainMessage() code that is in charge of operator scheduling chooses an operator pod uniformally:

/**
 * @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)));
/**
 * @dev divide by total number of pods, use modulus/remainder
 */
uint256 pod = random % _operatorPods.length;

As a consequence, operators of different tiers make the same yield but have different bonded amount requirement and risk, making the platform uncompetitive.

Impact

Operator incentivization is not implemented properly, making it unlikely new operators will join and destabilization the network.

Tools Used

Manual audit

Recommended Mitigation Steps

Calculate the next pod to execute using a fairer distribution.

@code423n4 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 25, 2022
code423n4 added a commit that referenced this issue Oct 25, 2022
@gzeoneth
Copy link
Member

Duplicate of #434

@gzeoneth gzeoneth marked this as a duplicate of #434 Oct 30, 2022
@gzeoneth gzeoneth added the duplicate This issue or pull request already exists label Oct 30, 2022
@ACC01ADE ACC01ADE added disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") responded The Holograph team has reviewed and responded labels Nov 9, 2022
@ACC01ADE
Copy link

ACC01ADE commented Nov 9, 2022

Refer to #434 for explanation.

@trust1995
Copy link

Sorry about the dup, submitted the findings in the last second and accidentally sent twice.

@gzeoneth gzeoneth added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 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
@gzeoneth
Copy link
Member

Consider with #433

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate This issue or pull request already exists QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 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")
Projects
None yet
Development

No branches or pull requests

4 participants