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

Payable functions using delegatecall inside a loop

smartoshield

Summary

When calling delegatecall the same msg.value amount will be accredited multiple times.

Vulnerability Details

The contract TradingAccountBranch uses the delegatecall proxy pattern (which takes user-provided call data) in a payable function within a loop. This means that each delegatecall within the for loop will retain the msg.value of the transaction.

Impact

The protocol does not currently use the msg.value in any meaningful way.
The use of delegatecall within a loop, in the context of a payable function, can lead to the repeated crediting of the msg.value amount, potentially causing unexpected behavior or loss of funds.

Proof of Code

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
​
contract DelegatecallInLoop{
​
mapping (address => uint256) balances;
​
function badCall(address[] memory receivers) public payable {
for (uint256 i = 0; i < receivers.length; i++) {
address(this).delegatecall(abi.encodeWithSignature("addBalance(address)", receivers[i]));
}
}
​
function addBalance(address a) public payable {
balances[a] += msg.value;
}
​
}

When calling the function badCall the same msg.value amount will be accredited multiple times.

Tools Used

Slither/Aderyn/Solodit

Recommendations

The function called by delegatecall should not be payable / doesn't use msg.value.

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.