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

Unsafe use of `delegatecall` in `TradingAccountBranch::createTradingAccountAndMulticall`

tedox

Summary

Calling TradingAccountBranch::createTradingAccountAndMulticall allows users to delegatecall any contract and function by their choice with no restrictions meaning they can very easily drain the contract of its funds.

Vulnerability Details

By simply passing the proper bytes a user can call into their malicious contract which can simply use transferFrom to transfer all of the funds in the contract to another contract, since the caller of transferFrom will be the TradingAccountBranch contract the transaction will pass without any issues and all of the funds will be stolen.

function createTradingAccountAndMulticall(
bytes[] calldata data,
bytes memory referralCode,
bool isCustomReferralCode
)
external
payable
virtual
returns (bytes[] memory results)
{
uint128 tradingAccountId = createTradingAccount(referralCode, isCustomReferralCode);
​
results = new bytes[](data.length);
for (uint256 i; i < data.length; i++) {
bytes memory dataWithAccountId = bytes.concat(data[i][0:4], abi.encode(tradingAccountId), data[i][4:]);
@> (bool success, bytes memory result) = address(this).delegatecall(dataWithAccountId);
​
if (!success) {
uint256 len = result.length;
assembly {
revert(add(result, 0x20), len)
}
}
​
results[i] = result;
}
}

Impact

All funds in the contract can be stolen.

Tools Used

Manual review
VS Code
Aderyn

Recommendations

Ensure that the contract called is one of the approved contracts (e.g OrderBranch)

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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