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

Deposits can be maliciously bumped when oracle becomes stale/paused #36

Open
jokrsec opened this issue Dec 7, 2024 · 2 comments
Open
Labels
Low/Info A Low/Info severity issue.

Comments

@jokrsec
Copy link
Collaborator

jokrsec commented Dec 7, 2024

Summary

When the score oracle in BinaryEligibilityOracleEarningPowerCalculator.sol becomes stale or paused, _amountStaked is returned as the earning power instead of the old earning power. As a result, a keeper can bump all ineligible deposits to eligible deposits when the oracle becomes stale/paused and bump them back to ineligible once the oracle becomes fresh/unpaused.

Vulnerability Detail

  1. Let’s say there are 1,000 deposits delegated to ineligible delegatees, so their earning powers are currently zero.
  2. Due to some reason, the oracle becomes stale or paused.
  3. In this situation, the getNewEarningPower function of the binary eligibility calculator returns _amountStaked as the earning power instead of zero because of the following line:
    if (_isOracleStale() || isOraclePaused) return (_amountStaked, true);
  4. As a result, keepers can bump all these ineligible deposits, as their earning powers change from zero to non-zero, and collect bump tips.
  5. Once the oracle becomes normal again, those deposits can be bumped back, changing their earning powers from non-zero to zero, allowing keepers to extract bump tips again.

Impact

Deposits lose their rewards unfairly.

Code Snippet

function getNewEarningPower(
    uint256 _amountStaked,
    address, /* _staker */
    address _delegatee,
    uint256 /* _oldEarningPower */
  ) external view returns (uint256, bool) {
    // @audit-issue 
    if (_isOracleStale() || isOraclePaused) return (_amountStaked, true);

    if (!_isDelegateeEligible(_delegatee)) {
      bool _isUpdateDelayElapsed = (timeOfIneligibility[_delegatee] + updateEligibilityDelay) <= block.timestamp;
      return (0, _isUpdateDelayElapsed);
    }

    return (_amountStaked, true);
  }

Tool used

Manual Review

Recommendation

Return _oldEarningPower when oracle becomes stale or paused.

@jokrsec jokrsec changed the title Deposits Can Be Maliciously Bumped When Oracle Becomes Stale/Paused Deposits can be maliciously bumped when oracle becomes stale/paused Dec 7, 2024
@jokrsec jokrsec added the Medium A Medium severity issue. label Dec 7, 2024
@alexkeating
Copy link

We believe the severity should be lowered to low/info. In this scenario depositors that want to avoid the extra bump fee will have the updateEligibilityDelay to switch their delegatee before being bumped down again. As mentioned if the oracle becomes malicious oldEarningPower could be destructive to the system. For example, if the oracle gives earning power to a set of malicious delegates than depositors would be incentivized to delegate to those malicious delegates.

@jokrsec jokrsec added Low/Info A Low/Info severity issue. and removed Medium A Medium severity issue. labels Dec 13, 2024
@alexkeating
Copy link

Wont fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Low/Info A Low/Info severity issue.
Projects
None yet
Development

No branches or pull requests

2 participants