Rebalancing may not be able to sufficiently adjust the percentage

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-xeth/blob/d86fe0a9959c2b43c62716240d981ae95224e49e/src/AMO2.sol#L252 https://github.com/code-423n4/2023-05-xeth/blob/d86fe0a9959c2b43c62716240d981ae95224e49e/src/AMO2.sol#L299-L300

Vulnerability details

Impact

The Rebalance Defender may not be able to adequately rebalance.

Proof of Concept

There are contractual limitations on the rebalance operations. It is assumed that these are put in place to ensure that the Rebalance Defender bot is not allowed to misbehave, such that trust can be placed in the contract rather than having to trust an off-chain bot.

The percentage of xETH is supposed to lie between REBALANCE_DOWN_THRESHOLD and REBALANCE_UP_THRESHOLD.

AMO2.rebalanceUp() can only be called when the percentage of xETH is above REBALANCE_UP_THRESHOLD. This is supposed to burn xETH to bring the percentage back down to its proper range. However, rebalanceUp() is restricted by rebalanceUpCap which is set by the DEFAULT_ADMIN_ROLE. rebalanceUp() may therefore not be able to burn enough xETH such that the new percentage lies within the thresholds. Because of afterCooldownPeriod one insufficient rebalance operation cannot be immediately followed by another. Thus Rebalance Defender may not be able to restore the percentage.

The same is true, mutatis mutandis, for AMO2.rebalanceDown(), where xETH is minted to bring the percentage up.

Recommended Mitigation Steps

The rebalance caps may be removed in favour of making sure the amount to burn or mint brings the percentage back to its ideal value. The ideal amount to burn/mint could be calculated in these functions themselves. A cheaper option is to simply check that the new percentage is within the thresholds. preRebalanceCheck() may be reused for this. In the latter option an even narrower constraint might be also used, for example by checking that the new percentage is within a certain distance of an ideal percentage. This ideal percentage may either be set independently (in between the thresholds) or calculated as, for example, the half-way point between then thresholds.

Assessed type

Invalid Validation

[c4-judge]

kirk-baird marked the issue as duplicate of #35

[d3e4]

I don't think this is a duplicate of #26 and #35. That issue (#35) is about how rebalancing may be incorrectly performed, even if would be possible. This issue (#28) is about constraints which may prevent rebalancing from being correctly performed. These constraints (rebalancing caps) are not a factor in #35. Fixing #35 still leaves this issue (#28) unmitigated, and vice versa. #35 is about a lack of validation of rebalancing, #28 is about a "DoS" of rebalancing.

[c4-judge]

kirk-baird marked the issue as satisfactory

[kirk-baird]

I agree with @d3e4 that this issue is sufficiently different from #35 to justify it's own issue.

However, this issue is a DoS related issue that will only apply if parameters are configured incorrectly by the admin. Since these parameters can be reconfigured immediately to recover through setRebalanceUpCap() and setRebalanceDownCap() and there is negligible loss of funds during this period I consider it a low severity issue.

[c4-judge]

kirk-baird marked the issue as not a duplicate

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)

[c4-judge]

kirk-baird marked the issue as grade-b

[d3e4]

these parameters can be reconfigured immediately to recover through setRebalanceUpCap() and setRebalanceDownCap()

It is true that they can be reconfigured immediately, but there is also a cooldown period during which rebalancing is blocked after a rebalancing. This means that if an insufficient rebalancing has been called it cannot immediately be corrected. The cooldown period can be temporarily set to 1 block (not 0), but this may not be safe in the case of a dysfunctional or malicious defender.

I don't think the issue here is only that the caps might be incorrectly configured, but that their mere existence is a stumbling block for good rebalancing. What is an appropriate configuration may change, possibly rapidly, so having these caps will persistently put the defender at risk of not being able to keep up. Indeed, an effective mitigation of #35 is contingent on the mitigation of #28.

[kirk-baird]

I agree with your comments that these cap may present issue into the future but I still don't see this as a medium severity issue.

[c4-sponsor]

vaporkane marked the issue as sponsor acknowledged