Summary
A security vulnerability was recently discovered after some users reported small decreases in the rewards reported for their accounts in the explorer. Members of the core development team analyzed the vulnerability to understand its impact, implemented a fix, validated the fix and completed deployment of the fix around 5:00pm EST Thursday 1/28/21.
The vulnerability, if exploited, would have caused certain delegators’ earnings to be reduced under certain conditions. In practice, triggering this vulnerability would require direct interaction with the contracts or custom integrations with the contracts because both the explorer and the node software do not support the actions required to trigger the vulnerability. This vulnerability has been fixed and users do not need to worry about the vulnerability impacting them going forward. At the moment, aside from a small number of cases where users may observe nominal differences in their reward amounts, the core development team does not believe there have been major cases of compromised user value. The technical details of the vulnerability are shared below for informational purposes.
Technical Details
Impact
What kind of vulnerability is it? Who is impacted?
Description
This vulnerability can cause the delegators’ earnings to be reduced if a delegator delegated to the same transcoder claims earnings through a past round (i.e. less than the current round) and the transcoder has generated rewards and/or fees after that round.
The root cause of this vulnerability is that when earnings are claimed, the transcoder’s cumulativeRewardFactor and cumulativeFeeFactor for the end round of the claim are set to the cumulativeRewardFactor and cumulativeFeeFactor values for the transcoder’s lastRewardRound and lastFeeRound if either of the values are zero for the end round of the claim. This is not a problem when claiming earnings through the current round (which happens automatically with every staking transaction). However, this can be a problem when claiming earnings through an end round before the current round. In this scenario, if the transcoder’s lastRewardRound or lastFeeRound is after the end round, the BondingManager would store values based on rewards/fees generated in the future relative to the end round. As mentioned in a previous disclosure, the relationship between the start and end cumulative factors for a range of rounds determines the amount of rewards/fees to allocate to delegators. So, if the start cumulative factors are higher since they are based on rewards/fees generated in the future, delegators can end up being allocated less rewards/fees.
If a delegator triggers this vulnerability for round N, then the following outcomes are possible:
- The stake/fees calculated for a delegator with the same transcoder with
lastClaimRound = Nare less than what they should be. - The stake/fees calculated for a delegator with the same transcoder that has zero cumulative factors for the delegator’s
lastClaimRoundthat is within 100 rounds of N are less than what they should be.
Exploit Scenarios
This vulnerability could be inadvertently or maliciously triggered.
A delegator would need to call the claimEarnings() function on the BondingManager with an end round prior to the current round. However, the impact of this action depends on the lastClaimRound values of the other delegators for the transcoder. The impact would be higher if many delegators fulfill the criteria described earlier. The impact would be lower if few delegators fulfill the criteria described earlier.
Patches
Has the problem been patched? What versions should users upgrade to?
The vulnerability has been patched in the following set of commits:
Key Changes
Ensure that cumulative factors stored for future rounds cannot be cannot be stored for past rounds
This was a problem because when claiming earnings, the BondingManager might store the cumulative factors for the last round that a delegator’s transcoder called reward/earned fees for the end round of the claim. However, those cumulative factors could be for a future round relative to the end round of the claim.
This is addressed by:
- Only store cumulative factors for the end round of a claim if the last round that a delegator’s transcoder called reward/earned fees is before the end round of the claim.
- Prevent calling
claimEarnings()with an end round prior to the current round if the end round is after the LIP-36 upgrade round to make it impossible going forward for the end round of a claim to be before to the last round that a delegator’s transcoder called reward/earned fees.
Ensure that cumulative factors stored for future rounds cannot be used in stake/fee calculations for past rounds
This was a problem because when calling pendingStake() or pendingFees() with an end round before the current round, if the transcoder did not call reward/earn fees in the end round the cumulative factor values for that round would be 0. The BondingManager would then look up the cumulative factor values for the last round that the transcoder called reward/earned fees. But, these cumulative factor values could be for the future relative to the end round.
This is addressed by:
- Prevent calling
pendingStake()andpendingFees()with an end round prior to the current round if the end round is after the LIP-36 upgrade round which prevents cumulative factor values for future rounds to be used in stake/fee calculations for past rounds.
Ensure that when determining the starting cumulative factors when executing the LIP-36 earnings calculation algorithm for a range of rounds that the contract does not lookback past the LIP-36 upgrade round
This became a problem after a lookback was introduced in a previous bug fix. Since it is possible for future cumulative factor values to be stored for past rounds (see the first key change), it is also possible for the lookback process to go back to a past round that stores a future cumulative factor value.
This is addressed by:
- Stopping the lookback at the LIP-36 upgrade round because prior to that round we should not expect any stored cumulative factor values to be valid since LIP-36 had not be deployed yet.
The above solution corrects the stake/fee calculations for delegators with a last claim round before the LIP-36 upgrade round or a last claim round that is within MAX_LOOKBACK_ROUNDS (100) rounds of the LIP-36 upgrade round.
Non-Key Changes
pendingStake()/pendingFees() will include transcoder’s reward/fee cut even if the transcoder already claimed earnings for the round
Previously, if a transcoder claimed earnings for a round and then either called reward or earned fees, the reward/fee cut would not be included in the result returned by pendingStake() / pendingFees() . However, that reward/fee cut would be included in the result when the next round begins and the reward/fee cut could be claimed in the next round. Now, the reward/fee cut is included in the result returned by pendingStake() / pendingFees() even before that next round begins. Without this change, the transcoder would have still got that reward/fee cut. This change just makes sure the reward/fee cut is reflected in the result returned by pendingStake() / pendingFees() even before the next round begins.
Breaking Changes
These patches contain a few breaking changes. The latest changes attempt to minimize the amount of breakage and the following order listed of priorities is used to determine whether a particular breaking change is appropriate:
- Avoid unexpected state updates for users
- Avoid unexpected transaction reverts for users
- Avoid unexpected call return values for users
If one of these priorities comes into conflict with another, the higher ranked priority is preserved and the lower ranked priority is sacrificed.
claimEarnings() can only be called with the end round set to the current round or a round before the LIP-36 upgrade round
Sacrifice priority 2 to preserve priority 1. Rather than execute a state update based on a value that differs from the user provided value we revert early and give the user a reason for the failure.
pendingStake() always returns the calculated stake for the current round if the end round is >= the LIP-36 upgrade round
Partially sacrifice priority 3 in that the call return value still matches user expectations most of the time.
pendingStake() will still return the calculated stake for an end round before the LIP-36 upgrade round
Partially sacrifice priority 3 in that the call return value still matches user expectations most of the time.
Avoids breaking tools like merkle-earnings-cli which still need to call pendingStake() for a round prior to the LIP-36 upgrade round.
pendingFees() always returns the calculated fees for the current round if the end round is >= the LIP-36 upgrade round
Partially sacrifice priority 3 in that the call return value still matches user expectations most of the time.
pendingFees() will still return the calculated fees for an end round before the LIP-36 upgrade round
Partially sacrifice priority 3 in that the call return value still matches user expectations most of the time.
Avoids breaking tools like merkle-earnings-cli which still need to call pendingFees() for a round prior to the LIP-36 upgrade round.
Deployment
These were the steps taken for deployment with links to the relevant transactions:
- Deploy a new BondingManger implementation contract
- Stage an update with the Governor that registers the new BondingManager implementation with the Controller
- A delay of 0 was used because it was determined that a fix needed to be deployed as soon as possible to safeguard user value.
- Multisig submission of the tx
- Multisig approval and execution of the tx
- Execute the staged update to complete the registration of the new BondingManager implementation with the Controller
The list of deployed contract addresses has been updated with the latest version of the BondingManager implementation contract.
For more information
If you have any questions or comments about this advisory:
- Email us at security@livepeer.og