IntegralBaseLiquidityManager Manual Review Findings

IBL-01M: Absence of Rounding Towards Negative Infinity

Type	Severity	Location
Mathematical Operations		IntegralBaseLiquidityManager.sol:L548-L550

Description:

The IntegralBaseLiquidityManager::_checkVolatility does not round the time-weighted average tick towards negative infinity when it is negative in contrast to its OracleLibrary::consult implementation.

Impact:

As the purpose of the IntegralBaseLiquidityManager::_checkVolatility function is meant to evaluate a range of values, the impact of this exhibit is minimal and would only manifest in the condition that currentTick == twapTick - _maxTickChange, twapTick is negative, and twapTick rounded incorrectly. Combining the likelihood factors outlined and the actual impact, we consider this exhibit to be informational as it would permit volatility one tick greater than the enforced limit.

Example:

Copy
531/// @dev revert if volatility is above acceptable levels
532///      (mainly used to prevent flashloan attacks)
533/// @param currentTick Current pool tick
534function _checkVolatility(int24 currentTick) internal view {
535    // SLOADS for efficiency
536    uint32 _twapInterval = twapInterval;
537
538    int24 _maxTickChange = maxTickChange;
539
540    // Get TWAP tick
541    uint32[] memory secondsAgos = new uint32[](2);
542    secondsAgos[0] = _twapInterval;
543
544    // tickCumulatives is basically where the tick was as of twapInterval seconds ago
545    (int56[] memory tickCumulatives, ) = plugin.getTimepoints(secondsAgos);
546
547    // tickCumulatives[1] will always be greater than tickCumulatives[0]
548    int24 twapTick = int24(
549        (tickCumulatives[1] - tickCumulatives[0]) / int32(twapInterval)
550    );
551
552    // Make sure currentTick is not more than maxTickChange ticks away from twapTick
553    // No SafeMath here--even if a compromised governance contract set _maxTickChange to a very high value,
554    // it would only wrap around and cause this check to fail.
555    require(
556        currentTick <= twapTick + _maxTickChange &&
557            currentTick >= twapTick - _maxTickChange,
558        "V"
559    );
560}

Recommendation:

We advise the code to round towards negative infinity, similarly to the original Algebra Integral implementation.

Alleviation (e31556397b9d321737ff47809b572383dda4a2aa):

The code correctly rounds towards negative infinity per the original Algebra Integral implementation, ensuring that the TWAP tick values calculated are more accurate when negative.

IBL-02M: Non-Standard Disable of Initializers

Type	Severity	Location
Standard Conformity		IntegralBaseLiquidityManager.sol:L172

Description:

The Initializable::initializer modifier is utilized in the IntegralBaseLiquidityManager::constructor to disable the initializer at the base implementation, however, this is non-standard as the re-initialization pattern could still be utilized in the base implementation in theory.

Impact:

As initialization is still prohibited, the proposed pattern is a matter of best practice rather than an actual vulnerability.

Example:

Copy
172constructor() initializer {}

Recommendation:

We advise the Initializable::_disableInitializers function to be utilized instead, ensuring that the logic implementation is correctly disabled.

Alleviation (e31556397b9d321737ff47809b572383dda4a2aa):

The Steer Protocol team evaluated this exhibit and clarified that they do not utilize an OpenZeppelin version that exposes the Initializable::_disableInitializers function.

As such, the presently adopted pattern by the Steer Protocol team is correct rendering this exhibit nullified.

IBL-03M: Potentially Insecure Volatility Oracle Integration

Type	Severity	Location
Standard Conformity		IntegralBaseLiquidityManager.sol:L347-L348, L351-L352

Description:

Per the Algebra Integral AMM implementation, it is possible for the plugin of an AMM pool to change during its lifetime provided that this action has been taken by a pool administrator affiliated with the Algebra protocol.

If such a scenario occurs, the IntegralBaseLiquidityManager contract will point to an outdated plugin instead of the latest one by the pool that may still comply with the IVolatilityOracle interface.

Impact:

As the update of a pool's plugin represents a moderate-likelihood scenario (i.e. an update of the plugin to support more features), we consider that the vulnerability would manifest in the foreseeable future.

Example:

Copy
345// Set pool
346pool = IAlgebraPool(_pool);
347address _plugin = IAlgebraPool(_pool).plugin();
348require(_plugin != address(0));
349uint32[] memory secondsAgos = new uint32[](2);
350secondsAgos[0] = _twapInterval;
351IVolatilityOracle(_plugin).getTimepoints(secondsAgos); //Just to check if this pool has a oracle plugin if not reverts from here
352plugin = IVolatilityOracle(_plugin);

Recommendation:

We advise the code to dynamically evaluate the IAlgebraPool::plugin when needed, and to implement a "graceful" pause procedure in case the plugin no longer supports the IVolatilityOracle interface.

Alleviation (e31556397b9d321737ff47809b572383dda4a2aa):

The code was updated to dynamically fetch the plugin from the IAlgebraPool and to invoke the IVolatilityOracle::getTimepoints function using the try-catch syntax, gracefully handling an error that would indicate a fatal error by pausing the vault.

As such, we consider this exhibit fully alleviated.