Zaros Part 1

Summary

The createTradingAccountAndMulticall function uses delegatecall to execute multiple function calls in the context of the TradingAccountBranch contract. This can be dangerous if the input data is not properly sanitized, leading to potential security risks such as unauthorized state changes or fund transfers.

Vulnerability Details

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/TradingAccountBranch.sol#L285-L311

1. Craft Malicious Payload:

   The attacker needs to encode the function call to withdrawMargin with parameters that would transfer funds to their address.

1. Call createTradingAccountAndMulticall with Malicious Payload:

   The attacker prepares the data array containing the malicious payload and calls the createTradingAccountAndMulticall function.

• **Execution via **delegatecall:

  • The createTradingAccountAndMulticall function processes the data array.

  • It constructs the dataWithAccountId by appending the tradingAccountId to the malicious payload.

  • The delegatecall executes the malicious payload in the context of the TradingAccountBranch contract.

• Unauthorized Fund Transfer:

  • The withdrawMargin function is executed within the TradingAccountBranch contract's context.

  • The function transfers the specified amount of collateral from the contract to the attacker's address. Since delegatecall preserves the context, the state changes made by withdrawMargin directly affect the TradingAccountBranch contract.

Impact

• Unauthorized Fund Transfers

• State Manipulation

• Access Control Bypass

Tools Used

Manual Review

Recommendations

• Validate the data array to ensure only allowed functions can be called.

• Implement strict access control checks in all functions that can be called via delegatecall.

• Use reentrancy guards to prevent reentrant calls that could exploit the contract's state.