Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Attached ETH will be lost by calling createTradingAccountAndMulticall

0xlrivo

Summary

Calling createTradingAccountAndMulticall with ETH attached will result in a permanent loss of it.

Vulnerability Details

TradingAccountBranch:createTradingAccountAndMulticall() allow users to create a new trading account and, in the same transaction, perform additional operations on it via delegatecall.

The calldata are modified to include the newly created tradingAccountId as follows:

bytes memory dataWithAccountId = bytes.concat(data[i][0:4], abi.encode(tradingAccountId), data[i][4:]);
(bool success, bytes memory result) = address(this).delegatecall(dataWithAccountId);

So, the only public function on the contract that matches this calldata layout, is depositMargin().

Impact

Since depositMargin() isn't payable, any ETH attached to the call will be forever stuck in the contract.

Tools Used

Manual Review

Recommendations

Consider making createTradingAccountAndMulticall as non-payable since there is no need to transfer ETH to this contract.

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.