StrategyV5 Manual Review Findings

SV5-01M: Implementation & Documentation Mismatch

Type	Severity	Location
Logical Fault		StrategyV5.sol:L498

Description:

The inline documentation of the referenced statement denotes that:

only invest 90% of liquidity for buffered flows

However, the full As4626Abstract::available amount is utilized for the investment preview.

Impact:

The system is presently inefficient as no liquidation buffer is utilized and the documentation does not match the implementation of the code.

Example:

Copy
489/**
490 * @dev Preview the amounts that would be invested based on the given amount
491 * @param _amount Amount of asset to invest with
492 * @return amounts uint256[8] Previewed investment amounts for each input in asset
493 */
494function previewInvest(
495    uint256 _amount
496) public view returns (uint256[8] memory amounts) {
497    if (_amount == 0)
498        _amount = available(); // only invest 90% of liquidity for buffered flows
499    int256[8] memory excessInput = _excessInputLiquidity(invested() + _amount);
500    for (uint8 i = 0; i < inputLength; i++) {
501        if (_amount < 10) break; // no leftover
502        if (excessInput[i] < 0) {
503            uint256 need = _inputToAsset(excessInput[i].abs(), i);
504            if (need > _amount)
505                need = _amount;
506            amounts[i] = need;
507            _amount -= need;
508        }
509    }
510}

Recommendation:

We advise the code to properly utilize only 90% of the As4626Abstract::available amount to ensure that a buffer is permitted for potential liquidations that may occur.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The Astrolab DAO team evaluated this exhibit and identified that it represented a simple discrepancy between the latest code implementation and the in-line documentation that accompanies it.

Specifically, the "90%" threshold can be imposed via inputWeights rendering a flat reduction unnecessary and in reality inefficient in the latest system.

As such, we consider this exhibit as alleviated and have downgraded its severity to reflect a documentational discrepancy.

SV5-02M: Discrepancy of Liquidation Preview

Type	Severity	Location
Mathematical Operations		StrategyV5.sol:L471, L474, L476

Description:

The StrategyV5::previewLiquidate function will add the minimum between totalPendingAssetRequest() + allocated.bp(150) and allocated to the input _amount, however, the ensuing subtraction will fail if the _amount that results exceeds the allocated value.

Impact:

As the StrategyV5::previewLiquidate function is solely utilized by off-chain software, the impact of this flaw would solely translate to off-chain services and whether they handle revert errors of the StrategyV5::previewLiquidate function or not.

Example:

Copy
465/**
466 * @dev Preview the amounts that would be liquidated based on the given amount
467 * @param _amount Amount of asset to liquidate with (0 == totalPendingAssetRequest() + allocated.bp(100))
468 * @return amounts uint256[8] Previewed liquidation amounts for each input
469 */
470function previewLiquidate(
471    uint256 _amount
472) public view returns (uint256[8] memory amounts) {
473    uint256 allocated = invested();
474    _amount += AsMaths.min(totalPendingAssetRequest() + allocated.bp(150), allocated); // defaults to requests + 1% offset to buffer flows
475    // excessInput accounts for the weights and the cash available in the strategy
476    int256[8] memory excessInput = _excessInputLiquidity(allocated - _amount);
477    for (uint8 i = 0; i < inputLength; i++) {
478        if (_amount < 10) break; // no leftover
479        if (excessInput[i] > 0) {
480            uint256 need = _inputToAsset(excessInput[i].abs(), i);
481            if (need > _amount)
482                need = _amount;
483            amounts[i] = _assetToInput(need, i);
484            _amount -= need;
485        }
486    }
487}

Recommendation:

We advise the totalPendingAssetRequest() + allocated.bp(150) value to be added to the _amount directly, and the _amount to be consequently assigned to the minimum between the calculated value and the value of allocated ensuring that a subtraction overflow cannot occur.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

Our recommendation has been applied to the letter, incrementing the _amount by the relevant buffer (updated to 0.5% from 1.5% in the latest implementation) and then calculating the minimum between the new _amount and the allocated value.

SV5-03M: Insecure Casting Operations

Type	Severity	Location
Mathematical Operations		StrategyV5.sol:L419, L420, L448, L449

Description:

The referenced operations will cast a uint256 variable to its signed representation (int256) without proper bound checks.

Impact:

Any cast-based overflow operation will not be properly detected by the Solidity version utilized, potentially causing misbehaviours in the calculations referenced if the cast values manage to exceed the maximum of an int256.

Example:

Copy
407/**
408 * @dev Calculate the excess weight for a given input index
409 * @param _index Index of the input
410 * @param _total Total invested amount
411 * @return int256 Excess weight (/AsMaths.BP_BASIS)
412 */
413function _excessWeight(
414    uint8 _index,
415    uint256 _total
416) internal view returns (int256) {
417    if (_total == 0) _total = invested();
418    return
419        int256(invested(_index).mulDiv(AsMaths.BP_BASIS, _total)) -
420        int256(uint256(inputWeights[_index]));
421}

Recommendation:

We advise each cast to be performed safely, ensuring the value being cast is less-than the maximum supported by the int256 data type (i.e. type(int256).max).

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The Astrolab DAO team indicated that they plan to enforce safety checks for the relevant casting operations in a future iteration of the codebase per the linked GitHub discussion.

As such, we consider this exhibit to be safely acknowledged.