Maia DAO - Ulysses - 0xSmartContract's results

QA Report Issues List

• Low 01 → Head Overflow Bug in Calldata Tuple ABI-Reencoding
• Low 02 → Important _fee return value doesn’t check
• Low 03 → remoteBranchExecutionGas must be lower than gasLimit
• Low 04 → If the same values are used for status values in the same contract, this may cause logic errors
• Low 05 → Project Upgrade and Stop Scenario should be
• Low 06 → Array lengths doesn't check a few Struct
• Low 07→ Missing Event for initialize
• Non-Critical 08→ The function updates addresses in bulk. This can lead to unnecessary transaction costs and complexity.

</br> </br>

[Low-01] Head Overflow Bug in Calldata Tuple ABI-Reencoding

There is a known security vulnerability between versions 0.5.8 - 0.8.16 of Solidity, details on it below

The effects of the bug manifest when a contract performs ABI-encoding of a tuple that meets all of the following conditions: The last component of the tuple is a (potentially nested) statically-sized calldata array with the most base type being either uint or bytes32. E.g. bytes32[10] or uint[2][2][2]. The tuple contains at least one dynamic component. E.g. bytes or a struct containing a dynamic array. The code is using ABI coder v2, which is the default since Solidity 0.8.0.

https://blog.soliditylang.org/2022/08/08/calldata-tuple-reencoding-head-overflow-bug/

   3: pragma solidity ^0.8.0;

src\interfaces\IVirtualAccount.sol:
   7: struct Call {
   8:     address target;
   9:     bytes callData;
  10: }

src\VirtualAccount.sol:
  63  
  64  
  65:     /// @inheritdoc IVirtualAccount
  66:     function call(Call[] calldata calls) external override requiresApprovedCaller returns (bytes[] memory returnData) {
  67:         uint256 length = calls.length;
  68:         returnData = new bytes[](length);
  69: 
  70:         for (uint256 i = 0; i < length;) {
  71:             bool success;
  72:             Call calldata _call = calls[i];
  73: 
  74:             if (isContract(_call.target)) (success, returnData[i]) = _call.target.call(_call.callData);
  75: 
  76:             if (!success) revert CallFailed();
  77: 
  78:             unchecked {
  79:                 ++i;
  80:             }
  81:         }
  82:     }

Recommended Mitigation Steps

Because of this problem, I recommend coding the contract with fixed pragma instead of floating pragma for compiling with min 0.8.16 and higher versions. VirtualAccount contract can be compiled with floating pragma with all versions 0.8

</br>

[Low-02] Important _fee return value doesn’t check

RootBridgeAgent.getFeeEstimate() function calculate to estimate fee value, but doesn’t check to return _fee value for 0 value

0 value check can be doesn't important mostly but this function has critical return value.

src\RootBridgeAgent.sol:
  138      /// @inheritdoc IRootBridgeAgent
  139      /// @inheritdoc IRootBridgeAgent
  140:     function getFeeEstimate(
  141:         uint256 _gasLimit,
  142:         uint256 _remoteBranchExecutionGas,
  143:         bytes calldata _payload,
  144:         uint16 _dstChainId
  145:     ) external view returns (uint256 _fee) {
  146:         (_fee,) = ILayerZeroEndpoint(lzEndpointAddress).estimateFees(    // @audit _fee value is not check
  147:             _dstChainId,
  148:             address(this),
  149:             _payload,
  150:             false,
  151:             abi.encodePacked(uint16(2), _gasLimit, _remoteBranchExecutionGas, getBranchBridgeAgent[_dstChainId])
  152:         );
  153:     }

[Low-03] remoteBranchExecutionGas must be lower than gasLimit.

This situation is clearly stated in GasParams struck (remoteBranchExecutionGas must be lower than gasLimit.)

src\interfaces\BridgeAgentStructs.sol:
   7  
   8: struct GasParams {
   9:     uint256 gasLimit; // gas allocated for the cross-chain call.
  10:     uint256 remoteBranchExecutionGas; //gas allocated for remote branch execution. Must be lower than `gasLimit`.
  11: }

However, it is observed that this check is not made in many functions.

 function callOut(bytes calldata _params, GasParams calldata _gParams) external payable override lock {
        IBridgeAgent(localBridgeAgentAddress).callOut{value: msg.value}(payable(msg.sender), _params, _gParams);
    } // @audit _gParams value doesn’t check for lowet than



 function retrieveDeposit(uint32 _depositNonce, GasParams calldata _gParams) external payable override lock {
        // @audit _gParams value doesn’t check for lowet than
        if (getDeposit[_depositNonce].owner != msg.sender) revert NotDepositOwner();

        bytes memory payload = abi.encodePacked(bytes1(0x08), msg.sender, _depositNonce);

        _performCall(payable(msg.sender), payload, _gParams);
    }

Recommended Mitigation Steps

The remoteBranchExecutionGas should be less than gasLimit. There should be a check to ensure this condition is always met to avoid running out of gas during execution.

[Low-04] If the same values are used for status values in the same contract, this may cause logic errors.

Statuses are used in the same contract, but if the values are the same, this will be prone to errors, so use the Enum system

src\interfaces\BridgeAgentConstants.sol:
   8   */
   9   */
  10: contract BridgeAgentConstants {
  11:     // Settlement / Deposit Execution Status
  12: 
  13:     uint8 internal constant STATUS_READY = 0;
  14: 
  15:     uint8 internal constant STATUS_DONE = 1;
  20: 
  21:     uint8 internal constant STATUS_FAILED = 1;
  22: 
  23:     uint8 internal constant STATUS_SUCCESS = 0;

[Low-05] Project Upgrade and Stop Scenario should be

At the start of the project, the system may need to be stopped or upgraded, I suggest you have a script beforehand and add it to the documentation. This can also be called an " EMERGENCY STOP (CIRCUIT BREAKER) PATTERN ".

This can be done by adding the 'pause' architecture, which is included in many projects, to critical functions, and by authorizing the existing onlyOwner.

https://github.com/maxwoe/solidity_patterns/blob/master/security/EmergencyStop.sol

[Low-06] Array lengths doesn't check a few Struct

DepositMultipleInput , DepositMultipleParams , SettlementMultipleInput , SettlementMultipleParams structs has potential lengths check issue

The lengths of the hTokens, tokens, amounts, and deposits arrays should be consistent. An inconsistency can lead to out-of-bounds errors or incorrect operations.

A modifier or a check to ensure that all array lengths are equal.

src\interfaces\BridgeAgentStructs.sol:

  22: 
  29: 
  30: struct DepositMultipleInput {
  31:     //Deposit Info
  32:     address[] hTokens; //Input Local hTokens Address.
  33:     address[] tokens; //Input Native / underlying Token Address.
  34:     uint256[] amounts; //Amount of Local hTokens deposited for interaction.
  35:     uint256[] deposits; //Amount of native tokens deposited for interaction.
  36: }
  37: 
  38: struct DepositMultipleParams {
  39:     //Deposit Info
  40:     uint8 numberOfAssets; //Number of assets to deposit.
  41:     uint32 depositNonce; //Deposit nonce.
  42:     address[] hTokens; //Input Local hTokens Address.
  43:     address[] tokens; //Input Native / underlying Token Address.
  44:     uint256[] amounts; //Amount of Local hTokens deposited for interaction.
  45:     uint256[] deposits; //Amount of native tokens deposited for interaction.
  46: }

  73: 
  74: struct SettlementMultipleInput {
  75:     address[] globalAddresses; //Input Global hTokens Addresses.
  76:     uint256[] amounts; //Amount of Local hTokens deposited for interaction.
  77:     uint256[] deposits; //Amount of native tokens deposited for interaction.
  78: }
  79: 
  88: 
  89: struct SettlementMultipleParams {
  90:     uint8 numberOfAssets; // Number of assets to deposit.
  91:     address recipient; // Recipient of the settlement.
  92:     uint32 settlementNonce; // Settlement nonce.
  93:     address[] hTokens; // Input Local hTokens Addresses.
  94:     address[] tokens; // Input Native / underlying Token Addresses.
  95:     uint256[] amounts; // Amount of Local hTokens deposited for interaction.
  96:     uint256[] deposits; // Amount of native tokens deposited for interaction.
  97: }

[Low-07] Missing Event for initialize

Context:

10 results - 9 files

src\BaseBranchRouter.sol:
  59       */
  60:     function initialize(address _localBridgeAgentAddress) external onlyOwner {
  61          require(_localBridgeAgentAddress != address(0), "Bridge Agent address cannot be 0");

src\BranchPort.sol:
  121       */
  122:     function initialize(address _coreBranchRouter, address _bridgeAgentFactory) external virtual onlyOwner {
  123          require(coreBranchRouterAddress == address(0), "Contract already initialized");

src\CoreRootRouter.sol:
  82  
  83:     function initialize(address _bridgeAgentAddress, address _hTokenFactory) external onlyOwner {
  84          require(_setup, "Contract is already initialized");

src\MulticallRootRouter.sol:
  108       */
  109:     function initialize(address _bridgeAgentAddress) external onlyOwner {
  110          require(_bridgeAgentAddress != address(0), "Bridge Agent Address cannot be 0");

src\RootPort.sol:
  128       */
  129:     function initialize(address _bridgeAgentFactory, address _coreRootRouter) external onlyOwner {
  130          require(_bridgeAgentFactory != address(0), "Bridge Agent Factory cannot be 0 address.");

  146       */
  147:     function initializeCore(
  148          address _coreRootBridgeAgent,

src\factories\ArbitrumBranchBridgeAgentFactory.sol:
  55       */
  56:     function initialize(address _coreRootBridgeAgent) external override onlyOwner {
  57          require(_coreRootBridgeAgent != address(0), "Core Root Bridge Agent Address cannot be 0");

src\factories\BranchBridgeAgentFactory.sol:
  86       */
  87:     function initialize(address _coreRootBridgeAgent) external virtual onlyOwner {
  88          require(_coreRootBridgeAgent != address(0), "Core Root Bridge Agent cannot be 0");

src\factories\ERC20hTokenBranchFactory.sol:
  59       */
  60:     function initialize(address _wrappedNativeTokenAddress, address _coreRouter) external onlyOwner {
  61          require(_coreRouter != address(0), "CoreRouter address cannot be 0");

src\factories\ERC20hTokenRootFactory.sol:
  48       */
  49:     function initialize(address _coreRouter) external onlyOwner {
  50          require(_coreRouter != address(0), "CoreRouter address cannot be 0");

Description: Events help non-contract tools to track changes, and events prevent users from being surprised by changes Issuing event-emit during initialization is a detail that many projects skip

Recommendation: Add Event-Emit

[Non Critical-08] The function updates addresses in bulk. This can lead to unnecessary transaction costs and complexity.

The function checks addresses at startup to check for invalid addresses, this is good practice. However, this check must be repeated each time the address is updated.

Although this is not a security vulnerability, mass updating of addresses can increase the risk of accidentally entering an incorrect address. Additionally, this type of update may increase the possibility of a malicious user manipulating the system.

src\RootPort.sol:
  237      /// @inheritdoc IRootPort
  238      /// @inheritdoc IRootPort
  239:     function setAddresses(
  240:         address _globalAddress,
  241:         address _localAddress,
  242:         address _underlyingAddress,
  243:         uint256 _srcChainId
  244:     ) external override requiresCoreRootRouter {
  245:         if (_globalAddress == address(0)) revert InvalidGlobalAddress();
  246:         if (_localAddress == address(0)) revert InvalidLocalAddress();
  247:         if (_underlyingAddress == address(0)) revert InvalidUnderlyingAddress();
  248: 
  249:         isGlobalAddress[_globalAddress] = true;
  250:         getGlobalTokenFromLocal[_localAddress][_srcChainId] = _globalAddress;
  251:         getLocalTokenFromGlobal[_globalAddress][_srcChainId] = _localAddress;
  252:         getLocalTokenFromUnderlying[_underlyingAddress][_srcChainId] = _localAddress;
  253:         getUnderlyingTokenFromLocal[_localAddress][_srcChainId] = _underlyingAddress;
  254: 
  255:         emit LocalTokenAdded(_underlyingAddress, _localAddress, _globalAddress, _srcChainId);
  256:     }

Recommendation: Add stricter checks for address updates. For example, check if an address is already registered and only update new or changed addresses.

🛠️ Analysis - Maia DAO Ulysses Project

Summary

a) The approach I followed when reviewing the code

First, by examining the scope of the code, I determined my code review and analysis strategy. https://github.com/code-423n4/2023-09-maia

Accordingly, I analyzed and audited the subject in the following steps;

b) Analysis of the code base

The most important summary in analyzing the code base is the stacking of codes to be analyzed. In this way, many predictions can be made, including the difficulty levels of the contracts, which one is more important for the auditor, the features they contain that are important for security (payable functions, uses assembly, etc.), the audit cost of the project, and the time to be allocated to the audit; Uses Consensys Solidity Metrics

• Lines: total lines of the source unit
• nLines: normalized lines of the source unit (e.g. normalizes functions spanning multiple lines)
• nSLOC: normalized source lines of code (only source-code lines; no comments, no blank lines)
• Comment Lines: lines containing single or block comments
• Complexity Score: a custom complexity score derived from code statements that are known to introduce code complexity (branches, loops, calls, external interfaces, ...)

</br> </br>

Entry points

The diagram of the entry points where a user interacts with the protocol is the most practical and understandable way to first understand the protocol. In this diagram, the entry points of the project can be seen;

</br> </br>

RootBridgeAgent.sol

The Root Bridge Agent is responsible for sending/receiving requests to/from the LayerZero Messaging Layer for execution, as well as requests tokens clearances and tx execution from the RootBridgeAgentExecutor.

Bridge Agents allow for the encapsulation of business logic as well as the standardized cross-chain communication, allowing for the creation of custom Routers to perform actions as a response to remote user requests. This contract is for deployment in the Root Chain Omnichain Environment based on Arbitrum.

In order to understand the project, it is very important to generally understand the logic of Bridge Agents - Bridge Agents Executor and Router, which is the main working system;

c) Test analysis

What did the project do differently? ;

• 1. The tests of the project were designed according to the threat model and it was a part where the team did a great job.

What could they have done better?

• 1. It is important to have 100% test coverage, the slightest gap in a project's test coverage prevents unexpected major issues from being uncovered.

README.md:
  - What is the overall line coverage percentage provided by your tests?: 69%

</br>

• 2. The project utilizes Foundry as its development pipeline tool, containing an array of testsand scripts coded in Solidity. The Foundry tool automatically selects Solidity version 0.8.19 based on the version specified within the foundry.toml file. The project contains discrepancies with regards to the Solidity version used as the pragma statements of the contracts are open-ended ( ^0.8.0 ). We advise them to be locked to 0.8.19 ( =0.8.19 ), the same version utilized for our static analysis as well as optimizational review of the codebase.

</br> </br>

• 3. Test suites do not test for re-entrancy Test teams are testing many functions and variables, but recently, due to the vulnerability in the Vyper Compiler, the hacking of the projects using certain Vyper compiler and losing 50 million $ has revealed the security weakness here. https://cointelegraph.com/news/curve-finance-pools-exploited-over-24-reentrancy-vulnerability

lock modifier functions : 87 results - 12 files

The accuracy of the functions has been tested, but it has not been tested whether the lock modifier in the function works correctly or not. For this, a test must be written that tries to reentrancy and was observed to fail.

Let's take the depositToPor function from the project as an example, we can write any test of this function, it has already been written by the project teams in the test files, but the risk of reentrancy with the lock modifier in this function has not been tested, a test file can be added as follows;

Project File:

src\ArbitrumBranchBridgeAgent.sol:
  67       */
  68       */
  69:     function depositToPort(address underlyingAddress, uint256 amount) external payable lock {
  70:         IArbPort(localPortAddress).depositToPort(msg.sender, msg.sender, underlyingAddress, amount);
  71:     }

</br>

Reentrancy Test File:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {DSTest} from "forge-std//test.sol"
import {console2} from "forge-std/console2.sol";

import "./ArbitrumBranchBridgeAgent.sol"; // Replace with the path to your contract

contract ReentrancyAttack {
    ArbitrumBranchBridgeAgent victim;

    constructor(address _victim) {
        victim = ArbitrumBranchBridgeAgent(_victim);
    }

    // Fallback function to simulate reentrancy
    fallback() external payable {
        victim.depositToPort(address(0x0), 1 ether); // This should fail due to the lock modifier
    }

    function attack() external payable {
        victim.depositToPort(address(this), 1 ether); // This will trigger the fallback function
    }
}

contract TestReentrancyProtection is DSTest {
    ArbitrumBranchBridgeAgent ArbitrumBranchBridgeAgentInstance;
    ReentrancyAttack attacker;

    function setUp() public {
        ArbitrumBranchBridgeAgentInstance = new ArbitrumBranchBridgeAgent();
        attacker = new ReentrancyAttack(address(ArbitrumBranchBridgeAgentInstance));
    }

    function testReentrancyAttack() public {
        // Fund the attacker contract
        address(attacker).transfer(2 ether);

        // Try the reentrancy attack
        bool success = address(attacker).call(abi.encodeWithSignature("attack()"));

        // The reentrancy attack should fail due to the lock modifier
        assertFalse(success, "Reentrancy attack succeeded!");
    }
}

d) Architectural

Ulysses Protocol has two major components, the Virtualized and the Unified Liquidity Management. The latter being inspired by the Delta Algorithm put forward by Layer 0; https://www.dropbox.com/s/gf3606jedromp61/Delta-Solving.The.Bridging-Trilemma.pdf?dl=0

Ulyssess: It has two separate concepts as Virtualization and Unified Virtualization: Allows the use of the same unit token across different networks Unified: Allows merging the same volume tokens from different networks in the same ERC4626 vault It uses Anyswap in annyCall v7 to use these two architectures; https://docs.multichain.org/developer-guide/anycall-v7

Ports

The protocol's omnichain architecture has its foundations set around the Ports. These constitute the liquidity repositories which are in charge of retaining and managing capital deposited in each different chain where Ulysses is present.

How do they work? In short, a Port is in play whenever assets are deposited into or withdrawn from the Ulysses Omnichain Liquidity System, in effect serving as a vault with a single-asset pool for each omnichain token active in a given chain. When interacting with a Port to perform any of these actions, no protocol fees will be charged from user's assets.

There are two types of Ports:

1. Branch Ports
2. Root Port

e) Documents

- Philosophy - The Myth of Maia

The eldest of the seven nymphs that make up the constellation of Pleiades, Daughter of Atlas, who carries the world’s weight on his shoulders and mother of Hermes, messenger of the gods. Spending most her life alone inside a cave in the peak of Mount Kyllene, not much is known about Maia other than she was a shy Earth goddess, devoting her life to nurture and whose very name translated to “Nursing Mother” the embodiment of growth! 🌿

-Video explanation of the codes of the Project;

Video narration of the code of the project is effective, especially for auditors who want visual learning. Project Video

-Project details;

https://v2-docs.maiadao.io/ In the documentation, the link of the Hermes-Twitter account is broken at the bottom of the page, apart from that, the documents summarize the project in general terms.

- Ulysses

Ulysses is devided in two separate concepts: Virtualized and Unified Liquidity. Virtualized liquidity is made possible by using anycall v7 as our messaging layer and means that an asset deposited from a specific chain, is recognized as a different asset from the "same" asset but from a different chain (ex: arb ETH is different from mainnet ETH). Unified Liquidity then unifies these tokens using a stableswap AMM and then depositing them in a Unified Liquidity Token, which is a Multi-Asset ERC4626 tokenized vault. This allows users to deposit any asset from any chain and receive a 1:1 representation of the underlying assets. These Unified Liquidity Tokens can then be used in any other protocol, like Uniswap v3.

All Maia Dao Projects-Documents Links; Twitter - MaiaDAOEco Twitter - HermesOmnichain Discord - Maia DAO Ecosystem Telegram - Maia DAO Medium - Maia DAO Website - Maia DAO Website - Hermes Maia Dao Documents Hermes Documents Governance - Proposal Maia DAO

f) Centralization risks

• DAO Mechanism

It is observed that the DAO proposals of the project are voted by a small number of people, for example this can be seen in the proposal below.As the project is new, this is normal in DAO ecosystems, but the centrality risk should be expected to decrease over time;

https://snapshot.org/#/maiadao.eth/proposal/0x38d9ffaa0641eb494c20d2b034df321a6865bf8859487468e1583dd095837488

Centralization risk in the DOA mechanism is that the people who can submit proposals must be on the whitelist, which is contrary to the essence of the DAO as it carries the risk of a user not being able to submit a proposal in the DAO even if he has a very high stake.

We should point out that this problem is beyond the centrality risk and is contrary to the functioning of the DAO. Because a user who has a governance token to be active in the DAO may not be able to bid if he is not included in the whitelist (there is no information in the documents about whether there is a warning that he cannot make a proposal if he is not on the whitelist)

There is no information on how, under what conditions and by whom the While List will be taken. 1- In the short term; Clarify the whitelist terms and processes and add them to the documents, and inform the users as a front-end warning in Governance token purchases. 2- In the Long Term; In accordance with the philosophy of the DAO, ensure that a proposal can be made according to the share weight.

• Centralization risk in contracts

There is a risk of centrality in the project as the onlyOwner The owner role has a single point of failure and onlyOwner can use critical functions, posing a centralization issue. There is always a chance for owner keys to be stolen, and in such a case, the attacker can cause serious damage to the project due to important functions.

The code detail of this topic is specified in automatic finding; https://github.com/code-423n4/2023-09-maia/blob/main/bot-report.md#m03-the-owner-is-a-single-point-of-failure-and-a-centralization-risk

Reducing the onlyOwner centrality risk in blockchain projects is essential to enhance security, decentralization, and community trust. Relying too heavily on a single owner or administrator can lead to various issues, such as single points of failure, potential abuse of power, and lack of transparency. Here are some strategies to mitigate the onlyOwner centrality risk:

1-Multi-Signature (Multi-Sig) Wallets: Instead of having a single owner with full control, you can implement multi-signature wallets. Multi-sig wallets require multiple parties to sign off on transactions or changes to the smart contract, making it more decentralized and secure.

2-Timelocks and Governance Delays: Introduce timelocks or governance delays for critical operations or updates. This means that proposed changes need to wait for a specified period before being executed. During this period, the community can review and potentially veto the changes if they identify any issues.

3-Decentralized Governance Mechanisms: Implement decentralized governance mechanisms, such as DAOs (Decentralized Autonomous Organizations) or community voting systems. This allows token holders or stakeholders to have a say in important decisions, making the project more community-driven.

Great summary of GalloDaSballo with examples of centrality risk and how to reduce it in previous Code4rena audits; https://gist.github.com/GalloDaSballo/881e7a45ac14481519fb88f34fdb8837

g) Systemic risks

According to the information of the project, it is stated that it can be used in rollup chains and popular EVM chains.

README.md:
  - Does it use a side-chain?: true

We can talk about a systemic risk here because there are some Layer2 special cases. Some conventions in the project are set to Pragma ^0.8.0, allowing the conventions to be compiled with any 0.8.x compiler. The problem with this is that Arbitrum is Compatible with 0.8.20 and newer. Contracts compiled with these versions will result in a non-functional or potentially damaged version that does not behave as expected. The default behavior of the compiler will be to use the latest version which means it will compile with version 0.8.20 which will produce broken code by default.

i) Security Approach of the Project

Successful current security understanding of the project;

1 - First they did the main audit from a reputable auditing organization like Zellic and resolved all the security concerns in the report

2- They manage the 2nd audit process with an innovative audit such as Code4rena, in which many auditors examine the codes.

What the project should add in the understanding of Security;

1- By distributing the project to testnets, ensuring that the audits are carried out in onchain audit. (This will increase coverage)

2- After the project is published on the mainnet, there should be emergency action plans (not found in the documents)

3- Add On-Chain Monitoring System; If On-Chain Monitoring systems such as Forta are added to the project, its security will increase.

For example ; This bot tracks any DEFI transactions in which wrapping, unwrapping, swapping, depositing, or withdrawals occur over a threshold amount. If transactions occur with unusually high token amounts, the bot sends out an alert. https://app.forta.network/bot/0x7f9afc392329ed5a473bcf304565adf9c2588ba4bc060f7d215519005b8303e3

4- After the Code4rena audit is completed and the project is live, I recommend the audit process to continue, projects like immunefi do this. https://immunefi.com/

5- Pause Mechanism This is a chaotic situation, which can be thought of as a choice between decentralization and security. Having a pause mechanism makes sense in order not to damage user funds in case of a possible problem in the project.

6- Upgradability There are use cases of the Upgradable pattern in defi projects using mathematical models, but it is a design and security option.

i) Other Audit Reports and Automated Findings

Automated Findings: https://github.com/code-423n4/2023-09-maia/blob/main/bot-report.md

Slither Report: https://github.com/code-423n4/2023-09-maia/blob/main/slither.txt

Other Audit Reports (Zellic): Zellic Audit Report

While investigating the potential risks in the project, the past audit reports can give us serious insights, so they should be taken into account and analyzed in detail.

Zellic Audit Report - Ulysses Protocol

j) Gas Optimization

The project is generally efficient in terms of gas optimizations, many generally accepted gas optimizations have been implemented, gas optimizations with minor effects are already mentioned in automatic finding, but gas optimizations will not be a priority considering code readability and code base size

Permit2 can integrated into your project, this will reduce gas usage; https://medium.com/morpho-labs/how-permit2-improves-the-user-experience-of-morpho-1fdbfb5843fe https://github.com/Uniswap/permit2/issues/232

k) Packages and Dependencies Analysis 📦

l) Other recommendations

✅ The use of assembly in project codes is very low, I especially recommend using such useful and gas-optimized code patterns; https://github.com/dragonfly-xyz/useful-solidity-patterns/tree/main/patterns/assembly-tricks-1

✅ It is seen that the latest versions of imported important libraries such as Openzeppelin are not used in the project codes, it should be noted. https://security.snyk.io/package/npm/@openzeppelin%2Fcontracts/

✅A good model can be used to systematically assess the risk of the project, for example this modeling is recommended; https://www.notion.so/Smart-Contract-Risk-Assessment-3b067bc099ce4c31a35ef28b011b92ff#7770b3b385444779bf11e677f16e101e

✅ Due to onlyOwner powers; It is better to override renounceOwnership so that it cannot be used.

✅ Use the constant.sol pattern, if all constants are collected in one file this provides a more compact and systematic structure

✅My suggestion is to improve the naming of contracts, design high-level architectural resources such as diagrams, user and contract interaction flows from different participants' perspectives, and further expand documentation

✅ Economic risk audit; Having the economic models audited separately, as professional projects have done recently, will provide higher security. For example: @Risk_DAO project only performs economic structure inspections

✅ Formal specification & invariant testing ; Having formal specification & invariant testing done separately will provide higher security by experts on this subject. For example: @agfviggiano only does these checks

m) New insights and learning from this audit

1- I learned the concept of Virtualized liquidity and Arbitrary Cross-Chain Execution

2- I learned the concept of Layer0

n) Summary of Vulnerabilities

The list and details of all the results that I analyzed the project and shared in Code4rena are shared separately.

Audit Results

• Medium 01 → ERC1155 tokens coming to the 'VirtualAccount.sol' contract are stuck in the contract because there is no withdraw function
• Low 01 → Head Overflow Bug in Calldata Tuple ABI-Reencoding
• Low 02 → Important _fee return value doesn’t check
• Low 03 → remoteBranchExecutionGas must be lower than gasLimit
• Low 04 → If the same values are used for status values in the same contract, this may cause logic errors
• Low 05 → Project Upgrade and Stop Scenario should be
• Low 06 → Array lengths doesn't check a few Struct
• Low 07→ Missing Event for initialize
• Non-Critical 08→ The function updates addresses in bulk. This can lead to unnecessary transaction costs and complexity.

o) Time spent on analysis

I allocated about 6 days to the project, most of this time was spent with architectural examination and analysis.

Time spent:

24 hours