DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: high
Invalid

Reentrancy Vulnerability in `createTradingAccountAndMulticall` Function in the `TradingBranchAccount` contract

Summary : The createTradingAccountAndMulticall function in the TradingAccountBranch contract is vulnerable to reentrancy attacks due to the use of delegatecall within a loop inside a payable function. This can allow an attacker to repeatedly call back into the contract, potentially causing unintended state changes and draining funds.

Vulnerability Details: The vulnerability arises because the createTradingAccountAndMulticall function uses delegatecall within a loop. Delegatecall executes code from another context but retains storage, msg.sender, and msg.value from its caller's context. When combined with being marked as payable, this setup allows for potential reentrant calls that could manipulate or drain funds

Code Reference:

createTradingAccountAndMulticall(
bytes[] calldata data,
bytes memory referralCode,
bool isCustomReferralCode
)
external
payable
{
uint128 trading AccountId = create Trading Account (referral Code,is Custom Referral Cod e);
results = new byte s[](data.length);
for(uint256 i; i<data.length;i++){
byte smem ory da taW ithA ccount Id=byte s.con cat(da ta[i][0:4],abi.encode(tradingAccoun tId),dat [i] [4:]);
(bool succe ss,b ytes memo ry resul t)=add ress(this).deleg atec all(data With Acc ountI d);
if(!success){
uint25 6len=resu lt.le ngth;
asse mbly { reve rt(add(result ,0x20 ),le n)}
}
result s[i]=res ult;
}
}

Impact:

One: Funds Drainage: Attackers can exploit this vulnerability by crafting payloads that recursively call back into functions transferring ether out of the contract.
Two: State Manipulation: Repeated entry points might lead inconsistent states across multiple invocations affecting overall system behaviour.

Tools Used: Manual Review & Slither

Proof Of Code: Run The Following test:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
import "forge-std/Test.sol";
import { TradingAccountBranch } from "../src/perpetuals/branches/TradingAccountBranch.sol";
contract ReentrancyAttack {
TradingAccountBranch public target;
bytes[] public data;
bytes public referralCode;
bool public isCustomReferralCode;
constructor(
TradingAccountBranch _target,
bytes[] memory _data,
bytes memory _referralCode,
bool _isCustomReferralCode
) {
target = _target;
data = _data;
referralCode = _referralCode;
isCustomReferralCode = _isCustomReferralCode;
}
fallback() external payable {
// Reenter the target contract
target.createTradingAccountAndMulticall(data, referralCode, isCustomReferralCode);
}
receive() external payable {}
function attack() external payable {
target.createTradingAccountAndMulticall{ value: msg.value }(data, referralCode, isCustomReferralCode);
}
}
contract TradingAccountBranchTest is Test {
TradingAccountBranch public tradingAccountBranch;
ReentrancyAttack public attacker;
function setUp() public {
tradingAccountBranch = new TradingAccountBranch();
bytes[] memory data = new bytes[](1);
data[0] = abi.encodeWithSignature("someFunction()");
bytes memory referralCode = "";
bool isCustomReferralCode = false;
attacker = new ReentrancyAttack(tradingAccountBranch, data, referralCode, isCustomReferralCode);
}
function testReentrancy() public {
vm.deal(address(attacker), 1 ether);
// Expect the call to revert due to reentrancy
vm.expectRevert();
attacker.attack{ value: 1 ether }();
}
}

Recommendations:

One: Use Checks Effects Interactions Pattern: Ensure proper ordering operations performed within functions minimizing risk introducing unintended side effects arising due improper handling delegated calls made recursively leading potential DoS situations mentioned earlier

Two: Make use of @openzeppelin Reentrancy Guard.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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