DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

Locked ETH in TradingAccountBranch.sol

Summary

This report details a security vulnerability found within the TradingAccountBranch contract of the provided Solidity codebase. The vulnerability pertains to the handling of Ethereum (ETH) payments, specifically instances where ETH can be received but not withdrawn. This issue could potentially allow users to deposit funds into the contract without being able to retrieve them, leading to a loss of funds.

Vulnerability Details

The vulnerability arises due to the absence of a withdrawal mechanism for ETH in the TradingAccountBranch contract. While the contract allows for the deposit of various types of tokens through the depositMargin function, there is no corresponding functionality to withdraw these tokens. This limitation applies to ETH as well, despite the contract being marked as payable, allowing it to receive ETH.

Impact

The primary impact of this vulnerability is financial loss for users who deposit ETH into the contract expecting to be able to withdraw it later. Since there is no withdrawal mechanism implemented, users would be unable to retrieve their ETH, rendering the contract unusable for its intended purpose of handling withdrawals alongside deposits.

Tools Used

The analysis was conducted manually, utilizing standard Solidity knowledge and best practices for smart contract development and auditing.

Recommendations

To mitigate this vulnerability, one of the following actions should be taken:

  1. Implement a Withdrawal Mechanism: Add a withdrawal function to the TradingAccountBranch contract that allows users to withdraw their ETH along with other supported tokens. This function should ensure that the withdrawal amount does not exceed the user's deposited balance and adhere to any necessary safety checks, such as maintaining minimum margin requirements.

  2. Remove Payable Attribute: If the intention is to restrict the contract to only accept deposits and not handle withdrawals, the payable attribute should be removed. However, this approach limits the contract's utility and may not align with the intended design.

It is crucial to carefully consider the contract's intended use case and choose the most appropriate action to either enable withdrawals or adjust the contract's design accordingly.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`createTradingAccountAndMulticall` shouldn't be payable

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.