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

payable Modifier in TradingAccountBranch::createTradingAccountAndMulticall

frontrunner

Summary

The createTradingAccountAndMulticall function is marked as payable, which means it can receive Ether (ETH) when called. However, the function's code does not utilize msg.value, the parameter that represents the amount of ETH sent with the call.

Vulnerability Details

function createTradingAccountAndMulticall(
bytes[] calldata data,
bytes memory referralCode,
bool isCustomReferralCode
)
external
- payable // Marked as payable but no msg.valu utilization
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

The payable modifier suggests that the function expects ETH to be sent with the call. This can be misleading to users and developers who might assume that sending ETH has some effect within the function.

Since the function is marked as payable but does not use msg.value, any ETH sent to this function call will not be utilized within the function.

Tools Used

Recommendations

If the function is not designed to handle ETH, removing the payable modifier will prevent ETH from being sent and eliminate potential confusion.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`createTradingAccountAndMulticall` shouldn't be payable

Appeal created

inallhonesty Lead Judge 10 months 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.