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

Current calculation of _getBasedBondAmount and _getCurrentBondAmount restricts the number of pods that an operator can bond in #256

Closed
Tracked by #88
code423n4 opened this issue Oct 25, 2022 · 2 comments
Labels
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 edited-by-warden invalid This doesn't seem right responded The Holograph team has reviewed and responded sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

code423n4 commented Oct 25, 2022

Lines of code

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

Vulnerability details

Impact

Based bond amount is calculated with the pod as the power factor. This restricts the number of pods available for operators when trying to join a pod even though there can be unlimited pods created because the bond amount gets too high.

Proof of Concept

In pod 1, operators pay 2 ** 0 * 100 * (10 ** 18), as mentioned in the docs.

  function _getBaseBondAmount(uint256 pod) private view returns (uint256) {
    return (_podMultiplier**pod) * _baseBondAmount;
  }

whereby _podMultiplier = 2
pod = 1 , podIndex = 0
_baseBondAmount = 100 * (10**18)

Deducting the decimal places, operators pay 100 tokens to join pod 1, which is not bad.

However, in pod 20, operators pay 2 ** 21 * 100 * (10 ** 18) to join, which evaluates to 104,857,600 tokens, deducting decimal places. Since the total supply of tokens is only 10B, this amount is pretty large and most operators can't possibly pay that much, which results in a redundancy of pods.

Tools Used

Remix IDE

Recommended Mitigation Steps

Consider a more gradual calculation of _getBasedBondAmount to accommodate more pods and operators or limit the number of pods available.

@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 gzeoneth added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Oct 31, 2022
@gzeoneth
Copy link
Member

Looks like design choice by await sponsor comment.

@alexanderattar alexanderattar added the responded The Holograph team has reviewed and responded label Nov 8, 2022
@alexanderattar
Copy link

Design choice. Save gas by not running such calculations and math, naturally limit number of pods. But also since bonding minimum might change at any time, by not setting a hard limit, the number of possible pods will adjust dynamically as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 edited-by-warden invalid This doesn't seem right responded The Holograph team has reviewed and responded sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

3 participants