As4626 Manual Review Findings

A62-01M: Discrepancy of Access Control

Type	Severity	Location
Logical Fault		As4626.sol:L231, L249, L266, L284-L289

Description:

The As4626::withdraw and As4626::redeem functions prevent invocation if the _owner of the shares being withdrawn is not the msg.sender, however, their safe-prefixed counterparts do not perform such validation.

Additionally, the As4626::_withdraw implementation properly supports allowance consumptions so the presence of access control is contradictory.

Impact:

While the discrepancy itself does not result in any vulnerability due to proper allowance management in the As4626::_withdraw function, we still consider it to be a non-informational issue in the code as it could have had a significant impact to its security.

Example:

Copy
218/**
219 * @notice Withdraw by burning the equivalent _owner's shares and sending _amount of asset to _receiver
220 * @dev Beware, there's no slippage control - use safeWithdraw if you want it
221 * @param _amount Amount of asset tokens to withdraw
222 * @param _receiver Who will get the withdrawn assets
223 * @param _owner Whose shares we'll burn
224 * @return shares Amount of shares burned
225 */
226function withdraw(
227    uint256 _amount,
228    address _receiver,
229    address _owner
230) external whenNotPaused returns (uint256) {
231    if (_owner != msg.sender) revert Unauthorized();
232    return _withdraw(_amount, previewWithdraw(_amount, _owner), _receiver, _owner);
233}
234
235/**
236 * @notice Withdraw assets denominated in asset
237 * @dev Overloaded version with slippage control
238 * @param _amount Amount of asset tokens to withdraw
239 * @param _receiver Who will get the withdrawn assets
240 * @param _owner Whose shares we'll burn
241 * @return amount Amount of shares burned
242 */
243function safeWithdraw(
244    uint256 _amount,
245    uint256 _minAmount,
246    address _receiver,
247    address _owner
248) public whenNotPaused returns (uint256 amount) {
249    amount = _withdraw(_amount, previewWithdraw(_amount, _owner), _receiver, _owner);
250    if (amount < _minAmount) revert AmountTooLow(amount);
251}

Recommendation:

We advise access control to either be imposed on all variants of these functions or to be omitted entirely, either of which we consider an adequate resolution to this exhibit.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

Both functions and their safe-prefixed counterparts now properly support allowance-based authorization of the msg.sender, ensuring uniform behaviour across the functions by addressing this exhibit.

A62-02M: Improper Allowance Adjustment

Type	Severity	Location
Logical Fault		As4626.sol:L690-L694

Description:

An As4626::cancelRedeemRequest operation will consume the allowance between the _operator and the _owner only if the opportunityCost is greater than 0, however, the adjustment will be for the full shares amount.

Impact:

The present mechanism will most likely consume a higher allowance than it should incorrectly.

Example:

Copy
690// Adjust the operator's allowance after burning shares, only if the operator is different from the owner
691if (opportunityCost > 0 && _owner != msg.sender) {
692    uint256 currentAllowance = allowance(_owner, _operator);
693    _approve(_owner, _operator, currentAllowance - shares);
694}

Recommendation:

We advise this approach to be revised as it is presently invalid. The code should either revoke an approval equivalent to the opportunityCost, or should unconditionally revoke the full approval of the shares.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The approval adjustment properly utilizes the opportunityCost in the latest implementation, addressing this exhibit.

A62-03M: Improper Capture of Entry Fee

Type	Severity	Location
Mathematical Operations		As4626.sol:L130

Description:

Any deposit-related function that utilizes As4626::previewDeposit will suffer truncation in contrast to the As4626::previewMint function as the basis point percentages are applied in a rounding-prone way.

Impact:

Fees captured from deposits and their respective deposit amount may not sum up to the actual amount the user supplied due to truncation.

Example:

Copy
466/**
467 * @notice Previews the amount of shares that will be minted for a given deposit amount
468 * @param _amount Amount of asset tokens to deposit
469 * @param _receiver The future owner of the shares to be minted
470 * @return shares Amount of shares that will be minted
471 */
472function previewDeposit(uint256 _amount, address _receiver) public view returns (uint256 shares) {
473    return convertToShares(_amount, false).subBp(exemptionList[_receiver] ? 0 : fees.entry);
474}

Recommendation:

The flaw arises from the following misconception:

We advise the basis-point related calculations to be streamlined across the codebase, ensuring that truncation is accounted for by utilizing the remainder of the amount after the fee's application.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The code has been refactored to no longer use the preview-related functions in the input _shares of the As4626::_deposit function, calculating the fee locally instead.

In this implementation, the actual deposited amount is calculated as the original amount minus the fee captured, ensuring that any truncation which may occur is solely reflected in the fee and does not impact the deposited amount.

As such, we consider this exhibit fully alleviated.

A62-04M: Improper Capture of Exit Fee

Type	Severity	Location
Mathematical Operations		As4626.sol:L267, L285

Description:

Any withdrawal-related function that utilizes As4626::previewRedeem will suffer truncation in contrast to the As4626::previewWithdraw function as the basis point percentages are applied in a rounding-prone way.

Impact:

Fees captured from withdrawals and their respective withdrawal amount may not sum up to the actual amount that the user is entitled to due to truncation.

Example:

Copy
253/**
254 * @notice Redeems/burns _owner's shares and sends the equivalent amount in asset to _receiver
255 * @dev Beware, there's no slippage control - you need to use the overloaded function if you want it
256 * @param _shares Amount of shares to redeem
257 * @param _receiver Who will get the withdrawn assets
258 * @param _owner Whose shares we'll burn
259 * @return assets Amount of assets withdrawn
260 */
261function redeem(
262    uint256 _shares,
263    address _receiver,
264    address _owner
265) external whenNotPaused returns (uint256 assets) {
266    if (_owner != msg.sender) revert Unauthorized();
267    return _withdraw(previewRedeem(_shares, _owner), _shares, _receiver, _owner);
268}
269
270/**
271 * @dev Overloaded version with slippage control
272 * @param _shares Amount of shares to redeem
273 * @param _minAmountOut The minimum amount of assets accepted
274 * @param _receiver Who will get the withdrawn assets
275 * @param _owner Whose shares we'll burn
276 * @return assets Amount of assets withdrawn
277 */
278function safeRedeem(
279    uint256 _shares,
280    uint256 _minAmountOut,
281    address _receiver,
282    address _owner
283) external whenNotPaused returns (uint256 assets) {
284    assets = _withdraw(
285        previewRedeem(_shares, _owner),
286        _shares, // _shares
287        _receiver, // _receiver
288        _owner // _owner
289    );
290    if (assets < _minAmountOut) revert AmountTooLow(assets);
291}

Recommendation:

The flaw arises from the following misconception:

We advise the basis-point related calculations to be streamlined across the codebase, ensuring that truncation is accounted for by utilizing the remainder of the amount after the fee's application.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The code has been refactored to no longer use the preview-related functions in the input _amount of the As4626::_withdraw function, calculating the fee locally instead.

In this implementation, the actual withdrawn amount is calculated as the original amount minus the fee captured, ensuring that any truncation which may occur is solely reflected in the fee and does not impact the withdrawn amount.

As such, we consider this exhibit fully alleviated.

A62-05M: Incorrect Estimation of Deposits

Type	Severity	Location
Mathematical Operations		As4626.sol:L452, L473

Description:

The As4626::previewDeposit function will incorrectly estimate the amount of shares the operation will result in as it will apply the entry fee on the shares minted rather than the tokens deposited, causing truncation issues to lead to different results.

Impact:

As shares will most likely have a lower accuracy than the assets deposited, the truncation will be more severe and thus underestimate the amount of shares that will be minted.

Example:

Copy
466/**
467 * @notice Previews the amount of shares that will be minted for a given deposit amount
468 * @param _amount Amount of asset tokens to deposit
469 * @param _receiver The future owner of the shares to be minted
470 * @return shares Amount of shares that will be minted
471 */
472function previewDeposit(uint256 _amount, address _receiver) public view returns (uint256 shares) {
473    return convertToShares(_amount, false).subBp(exemptionList[_receiver] ? 0 : fees.entry);
474}

Recommendation:

We advise the code to properly apply the entry fee to the input _amount, simulating the behaviour of the As4626::_deposit function.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The entry fee is correctly applied to the input _amount of the As4626::previewDeposit function in the latest implementation, addressing this exhibit.

A62-06M: Incorrect Estimation of Withdrawals

Type	Severity	Location
Mathematical Operations		As4626.sol:L495, L516

Description:

The As4626::previewWithdraw function will incorrectly estimate the amount of shares the operation will burn as it will apply the exit fee on the shares burned rather than the tokens withdrawn, causing truncation issues to lead to different results.

Impact:

As shares will most likely have a lower accuracy than the assets deposited, the truncation will be more severe and thus overestimate the amount of shares that will be burned.

Example:

Copy
487/**
488 * @notice Preview how many shares the caller needs to burn to get his assets back
489 * @dev You may get less asset tokens than you expect due to slippage
490 * @param _assets How much we want to get
491 * @param _owner The owner of the shares to be redeemed
492 * @return How many shares will be burnt
493 */
494function previewWithdraw(uint256 _assets, address _owner) public view returns (uint256) {
495    return convertToShares(_assets, true).revAddBp(exemptionList[_owner] ? 0 : fees.exit);
496}

Recommendation:

We advise the code to properly apply the exit fee to the output _amount, simulating the behaviour of the As4626::_withdraw function.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The exit fee is correctly applied to the input _amount of the As4626::previewWithdraw function in the latest implementation, addressing this exhibit.

A62-07M: Incorrect Maintenance of Allowances in Redemption Requests

Type	Severity	Location
Logical Fault		As4626.sol:L179, L670, L693

Description:

The creation of a redemption request will correctly ensure the caller (i.e. _operator) has been authorized to create the request (in case they are not the _owner themselves), however, the same approval will be incorrectly validated for cancellations as well as processing of these requests.

Impact:

A redemption request that was planned for another user can be trivially hijacked by the original _owner if they revoke their allowance, a trait we consider invalid in the system albeit with a small consequence.

Example:

Copy
579/**
580 * @notice Initiate a redeem request for shares
581 * @param _shares Amount of shares to redeem
582 * @param _operator Address initiating the request
583 * @param _owner The owner of the shares to be redeemed
584 */
585function requestRedeem(
586    uint256 _shares,
587    address _operator,
588    address _owner,
589    bytes memory _data
590) public nonReentrant whenNotPaused returns (uint256 _requestId) {
591    if (_operator != msg.sender || (_owner != msg.sender && allowance(_owner, _operator) < _shares))
592        revert Unauthorized();
593    if (_shares == 0 || balanceOf(_owner) < _shares)
594        revert AmountTooLow(_shares);
595
596    Erc7540Request storage request = req.byOwner[_owner];
597    if (request.operator != _operator) request.operator = _operator;
598
599    last.sharePrice = sharePrice();
600    if (request.shares > 0) {
601        if (request.shares > _shares)
602            revert AmountTooLow(_shares);
603
604        // reinit the request (re-added lower)
605        req.totalRedemption -= AsMaths.min(
606            req.totalRedemption,
607            request.shares
608        );
609        // compute request vwap
610        request.sharePrice =
611            ((last.sharePrice * (_shares - request.shares)) + (request.sharePrice * request.shares)) /
612            _shares;
613    } else {
614        request.sharePrice = last.sharePrice;
615    }
616
617    _requestId = ++requestId;
618    request.requestId = _requestId;
619    request.shares = _shares;
620    request.timestamp = block.timestamp;
621    req.totalRedemption += _shares;
622
623    if(_data.length != 0) {
624    // the caller contract must return the bytes4 value "0x0102fde4"
625        if(IERC7540RedeemReceiver(msg.sender).onERC7540RedeemReceived(_operator, _owner, _requestId, _data) != 0x0102fde4)
626            revert Unauthorized();
627    }
628    emit RedeemRequest(_owner, _operator, _owner, _shares);
629}
630
631/**
632 * @notice Initiate a withdraw request for assets denominated in asset
633 * @param _amount Amount of asset tokens to withdraw
634 * @param _operator Address initiating the request
635 * @param _owner The owner of the shares to be redeemed
636 * @param _data Additional data
637 * @return requestId The ID of the withdraw request
638 */
639function requestWithdraw(
640    uint256 _amount,
641    address _operator,
642    address _owner,
643    bytes memory _data
644) external returns (uint256) {
645    return requestRedeem(convertToShares(_amount, false), _operator, _owner, _data);
646}
647
648// /**
649//  * @notice Cancel a deposit request
650//  * @param operator Address initiating the request
651//  * @param owner The owner of the shares to be redeemed
652//  */
653// function cancelDepositRequest(
654//     address operator,
655//     address owner
656// ) external virtual nonReentrant {}
657
658/**
659 * @notice Cancel a redeem request
660 * @param _operator Address initiating the request
661 * @param _owner The owner of the shares to be redeemed
662 */
663function cancelRedeemRequest(
664    address _operator,
665    address _owner
666) external nonReentrant {
667    Erc7540Request storage request = req.byOwner[_owner];
668    uint256 shares = request.shares;
669
670    if (_operator != msg.sender || (_owner != msg.sender && allowance(_owner, _operator) < shares))
671        revert Unauthorized();
672    if (shares == 0) revert AmountTooLow(0);
673
674    last.sharePrice = sharePrice();
675    uint256 opportunityCost = 0;
676    if (last.sharePrice > request.sharePrice) {
677        // burn the excess shares from the loss incurred while not farming
678        // with the idle funds (opportunity cost)
679        opportunityCost = shares.mulDiv(
680            last.sharePrice - request.sharePrice,
681            weiPerShare
682        ); // eg. 1e8+1e8-1e8 = 1e8
683        _burn(_owner, opportunityCost);
684    }
685
686    req.totalRedemption -= shares;
687    if (isRequestClaimable(request.timestamp))
688        req.totalClaimableRedemption -= shares;
689
690    // Adjust the operator's allowance after burning shares, only if the operator is different from the owner
691    if (opportunityCost > 0 && _owner != msg.sender) {
692        uint256 currentAllowance = allowance(_owner, _operator);
693        _approve(_owner, _operator, currentAllowance - shares);
694    }
695    request.shares = 0;
696    emit RedeemRequestCanceled(_owner, shares);
697}

Recommendation:

We advise the code to consume the allowance during a redemption request's creation, and to permit the creator of the request (i.e. caller of As4626::requestRedeem) to either cancel or claim the request.

Alleviation (59b75fbee1):

While allowance is properly consumed during the creation of a redemption request, allowance remains validated in the As4626::cancelRedeemRequest.

As a redemption request should be possible to cancel even if the original requester does not have any allowance anymore, we re-iterate our original advice to omit allowance checks and supplement it with a recommendation to apply the opportunityCost allowance adjustment opportunistically (i.e. if currentAllowance - opportunityCost < 0, allowance should be configured to 0).

Alleviation (efbeab6478):

The code has been alleviated per our recommendation, omitting the allowance related checks during redemption request cancellations and ensuring that the allowance of the _operator is reduced up to 0 safely.

A62-08M: Inexistent Protection Against Re-Initialization

Type	Severity	Location
Logical Fault		As4626.sol:L46-L62

Description:

The As4626::init function does not prevent against re-initialization, causing the timestamps of the last data entry to be corrupted as well as permitting the name, symbol, and decimals of the ERC20 representation of the contract to be adjusted post-deployment.

Impact:

A severity of minor has been assigned as the function is privileged, however, its impact is significant as fees can be lost and impersonation attacks can be performed.

Example:

Copy
39/**
40 * @dev Initializes the contract with the provided ERC20 metadata, core addresses, and fees.
41 * Only the admin can call this function.
42 * @param _erc20Metadata The ERC20 metadata including name, symbol, and decimals.
43 * @param _coreAddresses The core addresses including the fee collector address.
44 * @param _fees The fees structure.
45 */
46function init(
47    Erc20Metadata calldata _erc20Metadata,
48    CoreAddresses calldata _coreAddresses,
49    Fees calldata _fees
50) public virtual onlyAdmin {
51    // check that the fees are not too high
52    setFees(_fees);
53    feeCollector = _coreAddresses.feeCollector;
54    req.redemptionLocktime = 6 hours;
55    last.accountedSharePrice = weiPerShare;
56    last.accountedProfit = weiPerShare;
57    last.feeCollection = uint64(block.timestamp);
58    last.liquidate = uint64(block.timestamp);
59    last.harvest = uint64(block.timestamp);
60    last.invest = uint64(block.timestamp);
61    ERC20._init(_erc20Metadata.name, _erc20Metadata.symbol, _erc20Metadata.decimals);
62}

Recommendation:

We advise the function to prevent re-invocation via a dedicated variable, ensuring the contract cannot be re-initialized.

Alleviation (59b75fbee1):

The Astrolab DAO team specified that they intend to supply an initialized public bool that will prevent re-initialization, however, no such change has been incorporated in the codebase yet.

As such, we consider this exhibit open in the codebase's current state.

Alleviation (efbeab6478):

An if clause was introduced ensuring that the _initialized flag of the ERC20 parent implementation is false and reverting otherwise.

As such, we consider this exhibit properly alleviated.

A62-09M: Potentially Invalid Cancellation Assumption

Type	Severity	Location
Logical Fault		As4626.sol:L687, L688

Description:

The As4626::cancelRedeemRequest will update the totalClaimableRedemption data entry even if the redemption request has not been factored in a liquidation call as the As4626::isRequestClaimable function does not guarantee a liquidation has taken place.

Impact:

Redemption requests that should be able to be cancelled may result in an uncaught underflow due to an incorrect assumption in relation to whether a liquidation that satisfies the redemption has been performed or not.

Example:

Copy
687if (isRequestClaimable(request.timestamp))
688    req.totalClaimableRedemption -= shares;

Recommendation:

We advise the code to ensure a liquidation has happened after the request has occurred, ensuring that the totalClaimableRedemption has a high likelihood of incorporating the shares that were meant to be liquidated.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The conditional has been updated to ensure that the redemption amount has been factored in a liquidation call by validating whether the request's timestamp is older than the last liquidation that occurred.

A62-10M: Improper Accounting of Fees in Downward Price Action

Type	Severity	Location
Logical Fault		As4626.sol:L301, L304, L321

Description:

The As4626::_collectFees function will become permanently inaccessible if a downward price action has occurred while a non-zero claimableAssetFees value exists. Such an action will cause the As4626::_collectFees function to continue execution while all values yielded by the AsAccounting::computeFees function are 0, resulting in the accountedSharePrice being configured to 0.

This will cause any invocation of AsAccounting::computeFees to first compute a change equal to the share price itself (which is invalid), and then cause the code to yield a division-by-zero error due to attempting to calculate the profit.

Impact:

All fees will become permanently inaccessible if a downward action occurs for the share and at least a single withdrawal / deposit has occurred for the vault which is a highly likely scenario.

Example:

Copy
293/**
294 * @notice Trigger a fee collection: mints shares to the feeCollector
295 */
296function _collectFees() internal nonReentrant returns (uint256 toMint) {
297
298    if (feeCollector == address(0))
299        revert AddressZero();
300
301    (uint256 assets, uint256 price, uint256 profit, uint256 feesAmount) = AsAccounting.computeFees(IAs4626(address(this)));
302
303    // sum up all fees: feesAmount (perf+mgmt) + claimableAssetFees (entry+exit)
304    toMint = convertToShares(feesAmount + claimableAssetFees, false);
305
306    // do not mint nor emit event if there are no fees to collect
307    if (toMint == 0)
308        return 0;
309
310    emit FeeCollection(
311        feeCollector,
312        assets,
313        price,
314        profit, // basis AsMaths.BP_BASIS**2
315        feesAmount,
316        toMint
317    );
318    _mint(feeCollector, toMint);
319    last.feeCollection = uint64(block.timestamp);
320    last.accountedAssets = assets;
321    last.accountedSharePrice = price;
322    last.accountedProfit = profit;
323    last.accountedSupply = totalSupply();
324    claimableAssetFees = 0;
325}

Recommendation:

We advise either the AsAccounting::computeFees implementation to yield non-zero values with a zero feesAmount when returning early, or the As4626::_collectFees function to return early if just feesAmount is 0.

While we advise the former of the two to prevent time-based fees from accumulating when the share moves in a downward manner, different approaches can also be utilized such as waiting until the share price rebounds to the latest tracked one before charging fees.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The relevant function has been relocated to the StrategyV5Agent (StrategyV5Agent::_collectFees) contract and the relevant AsAccounting function has been renamed to AsAccounting::claimableDynamicFees.

In the renamed implementation, a case of no fees will properly yield the correct price as well as assets value, permitting the logic to function properly thus alleviating this exhibit in full.

A62-11M: Incorrect Implementation of EIP-7540

Type	Severity	Location
Logical Fault		As4626.sol:L585-L590, L625

Description:

The As4626 contract is meant to comply with the EIP-7540 standard, however, it deviates from it in both its interface as well as the implementations of the various functions as denoted in the standard.

As an example, the As4626::requestRedeem function will invoke the IERC7540RedeemReceiver::onERC7540RedeemReceived function on the msg.sender rather than the _owner.

Impact:

The As4626 is not compatible with the EIP-7540 standard, and one of the callbacks it performs during redemption requests is done so to the caller rather than the _owner which is invalid behaviour.

Example:

Copy
579/**
580 * @notice Initiate a redeem request for shares
581 * @param _shares Amount of shares to redeem
582 * @param _operator Address initiating the request
583 * @param _owner The owner of the shares to be redeemed
584 */
585function requestRedeem(
586    uint256 _shares,
587    address _operator,
588    address _owner,
589    bytes memory _data
590) public nonReentrant whenNotPaused returns (uint256 _requestId) {
591    if (_operator != msg.sender || (_owner != msg.sender && allowance(_owner, _operator) < _shares))
592        revert Unauthorized();
593    if (_shares == 0 || balanceOf(_owner) < _shares)
594        revert AmountTooLow(_shares);
595
596    Erc7540Request storage request = req.byOwner[_owner];
597    if (request.operator != _operator) request.operator = _operator;
598
599    last.sharePrice = sharePrice();
600    if (request.shares > 0) {
601        if (request.shares > _shares)
602            revert AmountTooLow(_shares);
603
604        // reinit the request (re-added lower)
605        req.totalRedemption -= AsMaths.min(
606            req.totalRedemption,
607            request.shares
608        );
609        // compute request vwap
610        request.sharePrice =
611            ((last.sharePrice * (_shares - request.shares)) + (request.sharePrice * request.shares)) /
612            _shares;
613    } else {
614        request.sharePrice = last.sharePrice;
615    }
616
617    _requestId = ++requestId;
618    request.requestId = _requestId;
619    request.shares = _shares;
620    request.timestamp = block.timestamp;
621    req.totalRedemption += _shares;
622
623    if(_data.length != 0) {
624    // the caller contract must return the bytes4 value "0x0102fde4"
625        if(IERC7540RedeemReceiver(msg.sender).onERC7540RedeemReceived(_operator, _owner, _requestId, _data) != 0x0102fde4)
626            revert Unauthorized();
627    }
628    emit RedeemRequest(_owner, _operator, _owner, _shares);
629}

Recommendation:

We advise either the code to be substantially updated to comply with the EIP-7540 standard, or to remove support of EIP-7540 and instead implement a custom EIP-7540 adaptation removing unnecessary traits such as the IERC7540RedeemReceiver callback.

We consider either of the two approaches as valid alleviations to this exhibit given that the EIP-7540 is not yet mature.

Alleviation (59b75fbee1):

The code was updated to accommodate for EIP-7540, however, the standard itself underwent an update in between the preliminary report and its revision.

As an example, the IERC7540::requestRedeem function definition denotes a receiver argument on which the callback should be performed on instead of the _owner.

EIP-7540 integration should be revised based on the latest implementation of the standard as of 04-15-2024, and as an extension to the aforementioned recommendation we advise the As4626::requestDeposit concept to be revised as it implements a dangerous polyfill in which the _receiver will expect state mutations as described in the EIP-7540 and this chapter in particular.

Alleviation (efbeab6478):

The contract was refactored to achieve EIP-7540 compliancy solely in relation to redemption requests due to size limitations the contract must abide by and the updates involved in the EIP-7540 standard itself.

We observed that EIP-7540 compliancy is still not achieved in two significant areas that pertain to redemption requests.

The first area of concern is the As4626::claimableRedeemRequest function and the fact that it does not align with the relevant EIP-7540 implementation as described here. As such, integrators will be unable to reliably invoke the As4626::claimableRedeemRequest function to assess the portion of funds that are claimable for a particular _owner.

The other area of concern is the As4626::requestRedeem function implementation itself, and the fact that it overwrites the previous _shares requested rather than incrementing them. Per the standard itself\:

Assumes control of shares from owner and submits a Request for asynchronous redeem. This places the Request in Pending state, with a corresponding increase in pendingRedeemRequest for the amount shares.

As the present mechanism will overwrite the previous request if done for the same receiver, it will not behave per the standard and thus break compliancy.

We advise both deviancies to be alleviated so as to ensure the contract is and remains EIP-7540 compliant.

As an additional comment, the As4626::totalpendingWithdrawRequest function is mistyped and should be corrected.

Alleviation (cf5194da53):

The Astrolab DAO team evaluated our follow-up review of the exhibit and proceeded with addressing the three concerns raised within it.

Specifically, a As4626::claimableRedeemRequest polyfill was introduced that complies with the EIP-7540 function signature, the As4626::totalpendingWithdrawRequest typographic mistake was corrected, and the As4626::requestRedeem function was refactored to treat the input _shares as an increment of the existing redeem request if it exists.

As all EIP-7540 related compatibility concerns have been addressed by the Astrolab DAO team, we consider this exhibit fully alleviated.

A62-12M: Inexistent Reservation of Shares

Type	Severity	Location
Logical Fault		As4626.sol:L591, L593, L619

Description:

The As4626::requestRedeem function will permit a user to request a redemption to be fulfilled at a later date. This request will reserve a portion of the available funds in the strategy and will cause a liquidation to occur to satisfy it.

The flaw in the current implementation is that a redemption request does not reserve the underlying EIP-20 balance, enabling a user to create multiple redemption requests with the same fungible EIP-20 balance across multiple accounts.

Impact:

It is possible to cause the strategy to no longer operate by creating multiple redemption requests that must be honoured by the system's liquidation mechanisms.

Example:

Copy
579/**
580 * @notice Initiate a redeem request for shares
581 * @param _shares Amount of shares to redeem
582 * @param _operator Address initiating the request
583 * @param _owner The owner of the shares to be redeemed
584 */
585function requestRedeem(
586    uint256 _shares,
587    address _operator,
588    address _owner,
589    bytes memory _data
590) public nonReentrant whenNotPaused returns (uint256 _requestId) {
591    if (_operator != msg.sender || (_owner != msg.sender && allowance(_owner, _operator) < _shares))
592        revert Unauthorized();
593    if (_shares == 0 || balanceOf(_owner) < _shares)
594        revert AmountTooLow(_shares);
595
596    Erc7540Request storage request = req.byOwner[_owner];
597    if (request.operator != _operator) request.operator = _operator;
598
599    last.sharePrice = sharePrice();
600    if (request.shares > 0) {
601        if (request.shares > _shares)
602            revert AmountTooLow(_shares);
603
604        // reinit the request (re-added lower)
605        req.totalRedemption -= AsMaths.min(
606            req.totalRedemption,
607            request.shares
608        );
609        // compute request vwap
610        request.sharePrice =
611            ((last.sharePrice * (_shares - request.shares)) + (request.sharePrice * request.shares)) /
612            _shares;
613    } else {
614        request.sharePrice = last.sharePrice;
615    }
616
617    _requestId = ++requestId;
618    request.requestId = _requestId;
619    request.shares = _shares;
620    request.timestamp = block.timestamp;
621    req.totalRedemption += _shares;
622
623    if(_data.length != 0) {
624    // the caller contract must return the bytes4 value "0x0102fde4"
625        if(IERC7540RedeemReceiver(msg.sender).onERC7540RedeemReceived(_operator, _owner, _requestId, _data) != 0x0102fde4)
626            revert Unauthorized();
627    }
628    emit RedeemRequest(_owner, _operator, _owner, _shares);
629}

Recommendation:

We advise the As4626::requestRedeem function to ensure that the _shares being submitted as part of the request are correctly locked and to prevent their transfer or usage until the request is either cancelled or fulfilled.

Alleviation (59b75fbee1):

The code was updated to overload the ERC20::transfer function, however, the ERC20::transferFrom function continues to permit shares meant for a request to be transferred.

Alleviation (efbeab6478):

The ERC20::transferFrom function has been overridden as well, ensuring that all EIP-20 transfer related functions correctly impose the pending redemption request amount limitation.