Security Vulnerability Disclosure (Fixed!): Delegator stake and fees could be inflated by delegating to transcoders under certain conditions

Protocol
1

Summary

A security vulnerability was recently discovered after both a delegator and members of the core development team observed cases where certain delegators had incorrectly inflated stake and/or fees. 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 4:13am EST Sunday 3/28/21. A second follow up fix to address additional discovered cases where incorrectly inflated fees were possible not covered by the first fix was deployed around 4:13am EST Thursday 4/1/21.

The vulnerability allowed delegators to incorrectly inflate their stake and/or fees when changing their delegation due to a missing math operation to adjust the level of precision for values used to calculate delegator rewards and fees in the BondingManager bondWithHint() function that was required after LIP-71. In most cases, triggering this vulnerability would inflate stake and/or fees to an extremely high value such that additional staking actions would trigger contract reverts due to overflows in math calculations. In a small number of cases, triggering this vulnerability could inflate stake/and or fees to a value that could be then withdrawn from the system. There was one identified case where inflated fees amounting to ~2.6 ETH was withdrawn from the system, but the system remained solvent and able to fully honor withdrawals of all fees and broadcasting funds due to excess ETH in the system that was unaccounted for that has accrued for the past few years from previous rounding and precision issues that have already been addressed.

The vulnerability has been fixed and users do not need to worry about the vulnerability impacting them going forward. The technical details of the vulnerability are shared below for informational purposes.

Technical Details

For clarity and ease of description, each of the fixes are designated a LIP number:

  • The first fix deployed on 3/28/21 is designated as LIP-77
  • The second fix deployed on 4/1/21 is designated as LIP-78

Note that a few changes in LIP-77 were superseded by changes in LIP-78. The key changes described below focus on the changes that are still in effect for each LIP. For a full list of changes including those from LIP-77 that were superseded by LIP-78, refer to the patches section.

LIP-77 Key Changes

Multiply cumulativeRewardFactor and cumulativeFeeFactor values stored before the LIP-71 round by RESCALE_FACTOR in bondWithHint()

Without this change when a delegator changes delegation, the bondWithHint() function could copy cumulativeRewardFactor and cumulativeFeeFactor values for a transcoder from a pre LIP-71 round and store them for the current round without multiplying the values by RESCALE_FACTOR. These values need to be multiplied by RESCALE_FACTOR so that all math calculations after the LIP-71 round use the higher level of precision added in LIP-71.

Multiply cumulativeRewardFactor values stored after the LIP-71 round by RESCALE_FACTOR if they are less than 10 ** 21

At the time of deployment, there were already a few cases in which cumulativeRewardFactor values for a transcoder from a pre LIP-71 round were stored for the current round without multiplying by RESCALE_FACTOR. This is addressed by checking if the cumulativeRewardFactor value is less than 10 ** 21 because if the cumulativeRewardFactor is less than 10 ** 21 then the contract knows that it was not previously multiplied by RESCALE_FACTOR since the minimum value for the cumulativeRewardFactor after multiplying by RESCALE_FACTOR is 10 ** 21.

Add a single use executeLIP77() function to BondingManager to revert corrupted state for a single account

At the time of deployment, there was a single account 0xB47D8F87c0113827d44Ad0Bc32D53823C477a89d that had corrupted state where its bondedAmount stored in the contract was an incorrectly inflated amount. In all other cases, accounts impacted by this vulnerability had incorrectly inflated stake and/or fees, but those values were calculated and not yet stored in the contract. In this case, the account took a staking action that caused the incorrectly calculated amount to be stored as its bondedAmount in the contract.

In order to revert this corrupted state, a single use executeLIP77() function was added to the BondingManager that set the bondedAmount stored in the contract for the account to an externally specified value. The externally specified value was calculated using a script that forked from mainnet right before the state corruption and calculated the correct value for the bondedAmount. This script is deterministic so it can be run by anyone to verify that the externally specified value for the call to executeLIP77() matches the correct value returned by the script. This function was called once during the LIP-77 deployment and cannot be called again.

LIP-78 Key Changes

Prior to the LIP-78 round, multiply cumulativeFeeFactor values stored after the LIP-71 round by RESCALE_FACTOR if they are less than 10 ** 6

As of the LIP-78 round, the only cumulativeFeeFactor values stored after the LIP-71 round that are less than 10 ** 6 are values stored as a result of the vulnerability that LIP-77 aimed to address. This fact was confirmed via on-chain data analysis. Thus, in these cases, the contract knows that the cumulativeFeeFactor was never properly multiplied by RESCALE_FACTOR. So, if this check passes, the cumulativeFeeFactor is multiplied by RESCALE_FACTOR.

Prior to the LIP-78 round, divide cumulativeFeeFactor values stored after the LIP-71 round by RESCALE_FACTOR if they are greater than LIP_78_MAX_CFF

As of the LIP-78 round, the only cumulativeFeeFactor values stored after the LIP-71 round that are greater than LIP_78_MAX_CFF = 10 ** 32 are values that were incorrectly multiplied by RESCALE_FACTOR more than once. This fact was confirmed via on-chain data analysis. Thus, in these cases, the contract knows that the cumulativeFeeFactor should be divided by RESCALE_FACTOR to compensate for the previous extra multiplication.

Patches

The vulnerability was patched by the following commits:

LIP-77

LIP-78

Deployment

A delay of 0 was used for all staged updates in the Governor because the fixes needed to be deployed as soon as possible to avoid loss of user value.

LIP-77 (i.e. the first fix)

  1. Deployed a new BondingManager target implementation contract
  2. Staged an update with the Governor that registers the new BondingManager target implementation with the Controller
  3. Executed the staged update to complete the registration of the new BondingManager target implementation
  4. Identified state corruption that needed to be reverted
  5. Staged an update with the Governor that pauses the Controller
  6. Executed the staged update to pause the Controller
  7. Deployed a new BondingManager target implementation contract
  8. Staged an update with the Governor that registers the new BondingManager target implementation, unpauses the Controller and calls the single use executeLIP77() function with a parameter value of 19237428264288812856072 on the BondingManager proxy
  9. Executed the staged update to complete the registration of the new BondingManager target implementation, unpause the Controller and call executeLIP77() on the BondingManager proxy

LIP-78 (i.e. the second fix)

LIP_78_ROUND was set to round 2109.

  1. Staged an update with the Governor that pauses the Controller
  2. Executed the staged update to pause the Controller
  3. Deployed a new BondingManager target implementation contract
  4. Staged an update with the Governor that calls setLIPUpgradeRound() on the RoundsManager with the parameter values 78 and LIP_78_ROUND, registers the new BondingManager target implementation with the Controller and unpauses the Controller
  5. Executed the staged update to call setLIPUpgradeRound() on the RoundsManager, complete the registration of the new BondingManager target implementation and unpause the Controller

For More Information

If you have any questions or comments about this advisory:

1 Like