Genius Contracts Re-Assessment - Shuttle Labs

The GeniusVaultCore contract inherits from ReentrancyGuardUpgradeable and UUPSUpgradeable but fails to call their respective initializer functions during initialization. This omission breaks the initialization chain required for proper contract behavior.

In GeniusVaultCore.sol:

function _initialize(
    address _stablecoin,
    address _admin,
    address _multicall,
    uint256 _rebalanceThreshold,
    address _priceFeed
) internal onlyInitializing {
    if (_stablecoin == address(0)) revert GeniusErrors.NonAddress0();
    if (_admin == address(0)) revert GeniusErrors.NonAddress0();

    __ERC20_init("Genius USD", "gUSD");
    __AccessControl_init();
    __Pausable_init();

   // Missing critical initializer calls:// __ReentrancyGuard_init();// __UUPSUpgradeable_init();

}

The __ReentrancyGuard_init() should initialize the ReentrancyGuard storage :

    function __ReentrancyGuard_init() internal onlyInitializing {
        __ReentrancyGuard_init_unchained();
    }

    function __ReentrancyGuard_init_unchained() internal onlyInitializing {
        ReentrancyGuardStorage storage $ = _getReentrancyGuardStorage();
        $._status = NOT_ENTERED;
    }

1. The ReentrancyGuard's internal status variable remains uninitialized, breaking the reentrancy protection mechanism. This directly impacts all functions using the nonReentrant modifier in the contract because the uninitialized ReentrancyGuard means the status variable remains at its default value (0), rather than being set to NOT_ENTERED (1). This effectively breaks all reentrancy protection in the contract. An attacker can perform reentrancy attacks on any function marked with the nonReentrant modifier since the guard's state checks will fail silently.

2. The inheritance chain initialization is incomplete, leading to a broken contract state.

This test can be added to GeniusVault.t.sol:

    //E forge test --match-test testSetup -vv
    function testSetup() public {
        console2.log("Reentrancy Status after setup = %s",VAULT.MockedReentrancyGuardView());
        console2.log("\t---> should be equal to 1");
        assertEq(VAULT.MockedReentrancyGuardView(),1,"Reentrancy guard should be 1");
    }

And this function to ReentrancyGuardUpgradeable.sol :

    function MockedReentrancyGuardView() public view returns (uint256) {
        ReentrancyGuardStorage storage $ = _getReentrancyGuardStorage();
        return $._status;
    }

Here is the result :

It is recommended to add the missing initializer calls in the _initialize function:

function _initialize(
    address _stablecoin,
    address _admin,
    address _multicall,
    uint256 _rebalanceThreshold,
    address _priceFeed
) internal onlyInitializing {
    if (_stablecoin == address(0)) revert GeniusErrors.NonAddress0();
    if (_admin == address(0)) revert GeniusErrors.NonAddress0();

    __ERC20_init("Genius USD", "gUSD");
    __AccessControl_init();
    __Pausable_init();
    __ReentrancyGuard_init();// Add this line
    __UUPSUpgradeable_init();// Add this line

SOLVED: Initializers are now triggered within the contracts.

The swapToStables() function in GeniusMultiTokenVault.sol implements a weak slippage control mechanism that accepts any positive increase in balance after a swap, regardless of how minimal the increase is:

function swapToStables(
    address token,
    uint256 amount,
    address target,
    bytes calldata data
) external override onlyOrchestrator whenNotPaused {

uint256 preSwapBalance = stablecoinBalance();

if (token == NATIVE) {
    PROXYCALL.execute{value: amount}(target, data);
} else {
    IERC20(token).safeTransfer(address(PROXYCALL), amount);
    PROXYCALL.approveTokenExecute(token, target, data);
}

uint256 postSwapBalance = stablecoinBalance();

if (postSwapBalance <= preSwapBalance) revert GeniusErrors.TransferFailed(token, amount);

The issue lies in the comparison that only checks if postSwapBalance <= preSwapBalance. This permits swaps that return as little as 1 wei more than the initial balance to pass the validation.

MEV searchers can extract value from the protocol by:

1. Front-running swapToStables() transactions

2. Executing sandwich attacks on the swap

3. Returning the minimal amount required (preSwapBalance + 1) to pass the check

It also open doors for Orchestrator roles to drain the vault from any token token in exchange of 1 wei of stablecoin. The economic loss is bounded by the value of tokens being swapped minus 1 wei of stablecoin.

It is recommened to implement a proper slippage protection by adding a minAmountOut parameter and enforcing it:

function swapToStables(
    address token,
    uint256 amount,
    uint256 minAmountOut,
    address target,
    bytes calldata data
) external override onlyOrchestrator whenNotPaused {
    // ... existing checks ...

    uint256 preSwapBalance = stablecoinBalance();

    // ... execute swap ...

    uint256 postSwapBalance = stablecoinBalance();
    uint256 amountReceived = postSwapBalance - preSwapBalance;

    if (amountReceived < minAmountOut) {
        revert GeniusErrors.InvalidAmountOut(amountReceived, minAmountOut);
    }
}

SOLVED : A minAmountOut has been added to swapToStables() function.

The liquidity calculation logic within GeniusVaultCore's rebalancing mechanism can lead to user withdrawals being permanently blocked when maximum rebalancing occurs.

The issue stems from how minLiquidity() is calculated in relation to availableAssets(). When rebalancing takes place, it can consume all available liquidity without accounting for user withdrawal rights.

Relevant code:

// In GeniusVaultCore.sol
function availableAssets() public view returns (uint256) {
    uint256 _totalAssets = stablecoinBalance();
    uint256 _neededLiquidity = minLiquidity();
    return _availableAssets(_totalAssets, _neededLiquidity);
}

function _availableAssets(
    uint256 _totalAssets,
    uint256 _neededLiquidity
) internal pure returns (uint256) {
    if (_totalAssets < _neededLiquidity) {
        return 0;
    }
    return _totalAssets - _neededLiquidity;
}

// In GeniusVault.sol (but same in GeniusMultiTokenVault.sol
function minLiquidity() public view override returns (uint256) {
    uint256 _totalStaked = totalStakedAssets;
    uint256 reduction = (_totalStaked * rebalanceThreshold) / 10_000;
    uint256 minBalance = _totalStaked - reduction;
    return minBalance + claimableFees();
}

When rebalanceThreshold is set to 7500 (75%), and rebalanceLiquidity() is called with the maximum available amount, there is not enough funds for users to withdraw their stakes.

The same is true for fillOrder() function, meaning that an order can be filled with staked amounts from stakers.

This test can be added to GeniusVault.t.sol:

The issue is reproducible through the following sequence:

1. Users stake tokens (e.g., 200 USDC total)

2. Orchestrator rebalances 150 USDC (75% of total)

3. Remaining 50 USDC matches minimum required balance

4. All withdrawal attempts fail with InvalidAmount error

//E forge test --match-test "testStakeAndRebalance" -vv
    function testStakeAndRebalance() public {
        // Initial stakes from two users
        vm.startPrank(ALICE);
        deal(address(USDC), ALICE, 100 ether);
        USDC.approve(address(VAULT), 100 ether);
        VAULT.stakeDeposit(100 ether, ALICE);
        vm.stopPrank();

        vm.startPrank(BOB);
        deal(address(USDC), BOB, 100 ether);
        USDC.approve(address(VAULT), 100 ether);
        VAULT.stakeDeposit(100 ether, BOB);
        vm.stopPrank();

        // Verify initial state
        assertEq(VAULT.stablecoinBalance(), 200 ether, "Total vault balance should be 200 ether");
        assertEq(VAULT.totalStakedAssets(), 200 ether, "Total staked assets should be 200 ether");
        // Due to the 75% rebalanceThreshold (7_500 basis points), available assets should be 150 ether
        assertEq(VAULT.availableAssets(), 150 ether, "Available assets should be 150 ether");

        console2.log(" ----- Staking part done ----- ");

        // Setup rebalancing
        address bridgeAddress = makeAddr("bridge");
        uint256 amountToRebalance = 150 ether;
        bytes memory bridgeData = abi.encodeWithSelector(
            USDC.transfer.selector,
            bridgeAddress,
            amountToRebalance
        );

        // Execute rebalancing at the maximum amount possible
        vm.startPrank(ORCHESTRATOR);
        VAULT.rebalanceLiquidity(
            amountToRebalance,
            destChainId,
            address(USDC),
            bridgeData
        );
        vm.stopPrank();

        // Verify final state
        assertEq(VAULT.stablecoinBalance(), 50 ether, "Vault should have 100 ether remaining");
        assertEq(USDC.balanceOf(bridgeAddress), 150 ether, "Bridge should have received 100 ether");
        assertEq(VAULT.totalStakedAssets(), 200 ether, "Total staked assets should remain 200 ether");
        // Available assets = current balance - min required (200 * 0.25)
        assertEq(VAULT.availableAssets(), 0 ether, "Available assets should be 50 ether");

        
        console2.log(" ----- Rebalancing part done ----- ");
        console2.log("USDC amount available in the vault = %s",USDC.balanceOf(address(VAULT)));
        console2.log("Amount staked by Alice and Bob     = %s",VAULT.totalStakedAssets());

        console2.log("\n ----- Alice and Bob try to withdraw ");

        vm.startPrank(ALICE);
        VAULT.approve(address(VAULT), 100 ether);
        VAULT.stakeWithdraw(100 ether, address(ALICE), address(ALICE));
        vm.stopPrank();
        
        console2.log(" Alice could withdraw 100 ether");
        vm.startPrank(BOB);
        VAULT.approve(address(VAULT), 100 ether);
        VAULT.stakeWithdraw(100 ether, address(BOB), address(BOB));
        vm.stopPrank();
        console2.log(" Bob could withdraw 100 ether");
    }

This reverts because there is not enough funds in the contract:

It is recommended to have a rebalanceThreshold which allows staker to still withdraw their stakes when they want.

RISK ACCEPTED: Rebalancing action will handle this risk off-chain but no on chain requirement will be implemented.

In GeniusGasTank.sol, the sponsored transaction functions accept arbitrary fee tokens and amounts without proper validation mechanisms:

function sponsorOrderedTransactions(
    address target,
    bytes calldata data,
    IAllowanceTransfer.PermitBatch calldata permitBatch,
    bytes calldata permitSignature,
    address owner,
    address feeToken, //E @audit arbitrary
    uint256 feeAmount, //E @audit arbitrary
    uint256 deadline,
    bytes calldata signature
) external payable override whenNotPaused {
    // ... //
    IERC20(feeToken).safeTransfer(feeRecipient, feeAmount);
    // ... //
}

function sponsorUnorderedTransactions(
    address target,
    bytes calldata data,
    IAllowanceTransfer.PermitBatch calldata permitBatch,
    bytes calldata permitSignature,
    address owner,
    address feeToken, //E @audit arbitrary
    uint256 feeAmount, //E @audit arbitrary
    uint256 deadline,
    bytes32 seed,
    bytes calldata signature
) external payable override whenNotPaused {
    // ... //
    IERC20(feeToken).safeTransfer(feeRecipient, feeAmount);
    // ... //
}

In the Genius protocol, sponsored transactions allow users to define some parameters in order to "motivate" a sponsor to create a transaction for them.

However, since sponsors may not be the feeRecipient receiving fees for the sponsored transaction, the "incentivization" of higher fees = higher chance of having transactions sponsored is null, rendering the motivation for users who want to be sponsored to increase the fees of the transaction then increasing the chance that a sponsored transaction include 0 fee.

The lack of validation on fee tokens and amounts results in:

1. Non-standardized fee collection across the protocol

2. Acceptance of any ERC20 token as valid payment

3. Absence of minimum/maximum fee boundaries

4. Inconsistent economic incentives for transaction processing

It is recommended to implement strict fee validation by:

1. Adding a whitelist for accepted fee tokens.

2. Enforcing minimum and maximum fee amounts per token.

RISK ACCEPTED: The Shuttle Labs team accept the risk, as the transaction needs to be attractive (with good fees) to be validated and sponsored.

In GeniusMultiTokenVault.sol, the contract hardcodes the native token address as address(0), which creates incompatibility issues with certain L2 networks like zkSync where ETH has a specific contract address.

contract GeniusMultiTokenVault is IGeniusMultiTokenVault, GeniusVaultCore {
    using SafeERC20 for IERC20;

    // .... //

    address public immutable NATIVE = address(0);

This assumption breaks on networks like zkSync where ETH resides at 0x000000000000000000000000000000000000800A. The contract uses this NATIVE constant to identify the native token in several functions, including token balance checks and swap operations:

function tokenBalance(address token) public view returns (uint256) {
    if (token == NATIVE) {
        return address(this).balance;
    } else {
        return IERC20(token).balanceOf(address(this));
    }
}

Impact:

1. Native token operations fail on L2 networks where ETH has a non-zero address

2. Token balance checks return incorrect values for ETH

It is recommended to replace the hardcoded NATIVE address with a constructor parameter or implement a chain-specific configuration.

SOLVED: NATIVE is now an argument of the initialize() function.

The GeniusGasTank contract's sponsorOrderedTransactions function increments the user's nonce only at the end of the function during event emission, after performing external contract interactions. This implementation violates the Checks-Effects-Interactions pattern:

function sponsorOrderedTransactions(
    address target,
    bytes calldata data,
    IAllowanceTransfer.PermitBatch calldata permitBatch,
    bytes calldata permitSignature,
    address owner,
    address feeToken,
    uint256 feeAmount,
    uint256 deadline,
    bytes calldata signature
) external payable override whenNotPaused {
    if (deadline < block.timestamp) revert GeniusErrors.DeadlinePassed(deadline);

    bytes32 messageHash = keccak256(abi.encode(target,data,permitBatch,nonces[owner],feeToken,feeAmount,deadline,address(this)));

    _verifySignature(messageHash, signature, owner);
    address[] memory tokensIn = _permitAndBatchTransfer(
        permitBatch,
        permitSignature,
        owner,
        feeToken,
        feeAmount
    );
    IERC20(feeToken).safeTransfer(feeRecipient, feeAmount);

    if (target == address(PROXYCALL)) PROXYCALL.execute{value: msg.value}(target, data);
    else {
        PROXYCALL.approveTokensAndExecute{value: msg.value}(
            tokensIn,
            target,
            data
        );
    }

    emit OrderedTransactionsSponsored(msg.sender,owner,target,feeToken,feeAmount,nonces[owner]++);
}

The nonce increment occurs after multiple external interactions, including token transfers and proxy calls.

Impact:

1. State changes occur after external contract calls

2. The execution order breaks smart contract security best practices

3. Creates an unnecessary reentrancy vector despite the overall safety of the implementation

It is recommended to move the nonce increment before any external interactions:

function sponsorOrderedTransactions(
   // ... parameters ...
) external payable override whenNotPaused {
    if (deadline < block.timestamp) revert GeniusErrors.DeadlinePassed(deadline);

    bytes32 messageHash = keccak256(abi.encode(target,data,permitBatch,nonces[owner],feeToken,feeAmount,deadline,address(this)));
    _verifySignature(messageHash, signature, owner);

    uint256 currentNonce = nonces[owner];
    nonces[owner] = currentNonce + 1;// Increment nonce early

    address[] memory tokensIn = _permitAndBatchTransfer(
        permitBatch,
        permitSignature,
        owner,
        feeToken,
        feeAmount
    );

    // ... rest of the function ...

    emit OrderedTransactionsSponsored(msg.sender,owner,target,feeToken,feeAmount,currentNonce);
}

SOLVED: Nonce is now incremented after the verification of the signature.

The protocol implementation grants excessive privileges to administrative roles, enabling fund extraction through multiple vectors. This centralization manifests in three critical functions across the protocol's contracts:

1. In GeniusVaultCore.sol, the fillOrder function allows unrestricted execution by orchestrators:

   function fillOrder(
       Order memory order,
       address swapTarget,
       bytes memory swapData,
       address callTarget,
       bytes memory callData
   ) external virtual override nonReentrant onlyOrchestrator whenNotPaused {
      // No limit on fund movement through swap operations

   
   

2. In GeniusVaultCore.sol, rebalanceLiquidity provides access to substantial fund movement:

   function rebalanceLiquidity(
       uint256 amountIn,
       uint256 dstChainId,
       address target,
       bytes calldata data
   ) external payable virtual override onlyOrchestrator whenNotPaused {
       if (target == address(0)) revert GeniusErrors.NonAddress0();
       _isAmountValid(amountIn, availableAssets());
       // Can drain up to rebalanceThreshold

   
   

3. In GeniusMultiTokenVault.sol, swapToStables enables orchestrator-controlled token drainage:

   function swapToStables(
       address token,
       uint256 amount,
       address target,
       bytes calldata data
   ) external override onlyOrchestrator whenNotPaused {
      // Can drain non-stablecoin tokens by manipulating output validation

   
   

4. In GeniusVault.sol, withdrawFunds :

   function withdrawFunds() external onlyAdmin {
           uint256 balance = STABLECOIN.balanceOf(address(this));
           STABLECOIN.safeTransfer(msg.sender, balance);
       }

Impact:

1. The admin/orchestrator roles possess unilateral control over user funds

2. No time-locks or multi-signature requirements protect high-risk operations

3. A single compromised admin account can lead to complete protocol drainage

4. Users must place complete trust in the protocol administrators

It is recommended to add value limits for sensitive operations done by admins, implement multi-signature requirements.

ACKNOWLEDGED: The Shuttle Labs team acknowledge the issue and will switch to a governance as soon as possible.

The protocol contains several unused elements across multiple contracts that increase contract size and complicate code readability without providing functional value.

In GeniusVaultCore.sol, the _updateStakedBalance function exists but is never called:

function _updateStakedBalance(
    uint256 amount,
    uint256 add
) internal {
    if (add == 1) { totalStakedAssets += amount; }
    else { totalStakedAssets -= amount; }
}

The supportedBridges mapping in GeniusVaultCore.sol is declared but never utilized:

mapping(address => uint256) public supportedBridges;

In GeniusActions.sol, the authorizedOrchestrators mapping remains unused throughout the contract:

mapping(address => bool) internal authorizedOrchestrators;

Impact:

1. Increased deployment gas costs due to unnecessary bytecode

2. Reduced code maintainability and clarity

3. Higher complexity during security audits and code reviews

4. Misleading state variables suggesting unimplemented functionality

It is recommended to remove all unused code elements to improve contract efficiency and readability.

SOLVED: Unused code has been removed.

The GeniusVaultCore contract hardcodes price bounds with an assumed 8 decimals precision, even if it's the default behavior of Chainlink price feeds, it is not guaranteed that in the future price feeds use the same decimal configurations:

// Price bounds (8 decimals like Chainlink)
uint256 public constant PRICE_LOWER_BOUND = 98_000_000;// 0.98
uint256 public constant PRICE_UPPER_BOUND = 102_000_000;// 1.02

The price verification function uses these static bounds without accounting for the actual decimals returned by the price feed:

function _verifyStablecoinPrice() internal view returns (bool) {
    try stablecoinPriceFeed.latestRoundData() returns (
        uint80 roundId,
        int256 price,
        uint256 startedAt,
        uint256 updatedAt,
        uint80 answeredInRound
    ) {
        uint256 priceUint = uint256(price);
        if (priceUint < PRICE_LOWER_BOUND || priceUint > PRICE_UPPER_BOUND) {
            revert GeniusErrors.PriceOutOfBounds(priceUint);
        }

Impact:

1. Integration breaks with price feeds using non-8 decimal configurations

2. Manual deployment adjustments needed for different oracle implementations

3. Reduced protocol flexibility when integrating with new price feeds

It is recommended to initialize price bounds dynamically based on the price feed's decimals configuration:

contract GeniusVaultCore {
    uint256 public immutable PRICE_LOWER_BOUND;
    uint256 public immutable PRICE_UPPER_BOUND;

    function _initialize(
        address _stablecoin,
        address _admin,
        address _multicall,
        uint256 _rebalanceThreshold,
        address _priceFeed
    ) internal onlyInitializing {
        uint8 decimals = AggregatorV3Interface(_priceFeed).decimals();
        uint256 base = 10 ** decimals;

        PRICE_LOWER_BOUND = (98 * base) / 100;// 0.98 with proper decimals
        PRICE_UPPER_BOUND = (102 * base) / 100;// 1.02 with proper decimals

    // Rest of initialization...
    }
}

SOLVED: Price feeds can now be configured in initialize() function.