The use of delegatecall within a loop in a payable function can lead to security vulnerabilities. This pattern can cause the same msg.value amount to be accredited multiple times if used improperly, leading to incorrect accounting and potential exploits. It becomes especially dangerous if a future refactor introduces additional logic that relies on msg.value.
Proof of Concept
The code with the loop containing delegatecall is located in Multicall.sol.
This issue has similarities with a known vulnerability reported in vault-v2/contracts/Ladle.sol. The code snippet from this case demonstrates the use of delegatecall inside a loop, posing a high-severity risk:
function batch(bytes[] calldata calls) external payable returns (bytes[] memory results) {
results = new bytes[](calls.length);
for (uint256 i; i < calls.length; i++) {
(bool success, bytes memory result) = address(this).delegatecall(calls[i]);
if (!success) revert(RevertMsgExtractor.getRevertMsg(result));
results[i] = result;
}
}
In scenarios where msg.value impacts system arithmetic, an attacker can exploit this to manipulate balances without actually transferring corresponding funds.
Tools Used
The following tools were used for identifying and documenting the bug:
GitHub repository links for code inspection.
Reports of known vulnerabilities with similar patterns.
Recommended Mitigation Steps
Short-term: Document the risks associated with delegatecall within loops and ensure developers understand the potential attack vectors. This documentation should be incorporated into developer onboarding and training.
Long-term: Include security implications for all functions in the code documentation. Ensure future refactors or code updates do not introduce exploitable conditions by leveraging msg.value inappropriately.
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/multicall/Multicall.sol#L15
Vulnerability details
Impact
The use of
delegatecall
within a loop in a payable function can lead to security vulnerabilities. This pattern can cause the samemsg.value
amount to be accredited multiple times if used improperly, leading to incorrect accounting and potential exploits. It becomes especially dangerous if a future refactor introduces additional logic that relies onmsg.value
.Proof of Concept
delegatecall
is located in Multicall.sol.vault-v2/contracts/Ladle.sol
. The code snippet from this case demonstrates the use ofdelegatecall
inside a loop, posing a high-severity risk:msg.value
impacts system arithmetic, an attacker can exploit this to manipulate balances without actually transferring corresponding funds.Tools Used
The following tools were used for identifying and documenting the bug:
Recommended Mitigation Steps
delegatecall
within loops and ensure developers understand the potential attack vectors. This documentation should be incorporated into developer onboarding and training.msg.value
inappropriately.Assessed type
Context