Maia DAO Ecosystem - Qeew's results

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/base/TalosBaseStrategy.sol#L206-L207

Vulnerability details

Impact

In the deposit function within the TalosBaseStrategy contract, both slippage for two tokens (amount0Min and amount1Min) are hardcoded to zero. This can have severe implications as users may unintentionally accept a minimum of zero output tokens from a swap.

Proof of Concept

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/base/TalosBaseStrategy.sol#L206-L207.

As seen here, the hardcoding of zero slippage could potentially result in significant losses for users due to receiving no output tokens from the swap.

Tools Used

Manual Review

Recommended Mitigation Steps

It is advised to enable users to specify their preferred slippage. This will provide users with greater control reducing the risk of unexpected losses due to accepting swaps with no output tokens.

Assessed type

Other

Codebase Analysis

The Evaluation of the codebase was adopted through Two approach: Firstly, an in-depth review of the documentation paired with a thorough examination of the implementation within the code. Secondly, the previous audit by Zellic was also dissected to better comprehend their suggestions and ensure the corrections are implemented.

An intriguing issue surfaced from this evaluation: a particular fix linked to the 'delete' keyword in Solidity. The link to this fix is: https://github.com/Maia-DAO/maia-ecosystem-monorepo/commit/a0dd03114eee73e8be2bb822a626faaade5b6452.

In Solidity, the 'delete' keyword traditionally resets a variable back to its initial state. However, for a struct containing a mapping, Solidity does not provide a distinct "zero" state to reset the mapping. This is primarily because mapping keys can neither be iterated nor cleared out.

Yet, upon careful observation, the 'Deposit' struct seemed devoid of any mappings. Consequently, the 'delete' operation should function as anticipated - it should eliminate the deposit information from the storage and prohibit any potential repetitive fund withdrawals.

Architecture Evaluation The architecture adheres to the industry standards. It is well-structured and clearly outlined, showcasing a meticulous design process.

Centralization Risks There are notable centralization risks present, particularly surrounding the OnlyOwner role. This single entity has the power to make critical changes, which could lead to significant issues if the account were compromised.

Systemic Risks The project's reliance on the Anycall router by Multichain presents a substantial systemic risk. Recently, Multichain suspended services for several chains due to issues with its Router5 network. The team was unable to resolve these issues as they could not contact CEO Zhaojun, who was arrested, for necessary server access. More details on this situation can be found here: https://twitter.com/MultichainOrg/status/1663941611380965376.

Other Recommendations

While the provided documentation offers a helpful overview of the contract ecosystem, I found that details about Flywheel contracts are missing. This omission could create confusion for users trying to understand the technical aspects of the system. Therefore, I recommend the project team to prioritize providing clear and accurate information about these contracts.

Furthermore, I identified some mismatches between the comments and the actual code implementation, which could lead to confusion. It is essential to maintain consistency between the two to ensure better code readability and understanding.

Time Spent Over two weeks was spent on this

Time spent:

140 hours