code-423n4 / 2024-06-panoptic-findings

1 stars 0 forks source link

Use of delegatecall in a payable function inside a loop #31

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/Multicall.sol#L12-L36

Vulnerability details

Description

The Multicall contract uses the delegatecall proxy pattern (which takes user-provided calldata) 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. However, if a future version or refactor of the core protocol introduced a more meaningful use of it, it could be exploited to tamper with the system arithmetic.

Proof of Concept

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/Multicall.sol#L12-L36 /// @notice Performs multiple calls on the inheritor in a single transaction, and returns the data from each call. /// @param data The calldata for each call /// @return results The data returned by each call /// @audit-issue : No use of msg.value . function multicall(bytes[] calldata data) public payable returns (bytes[] memory results) { results = new bytes; for (uint256 i = 0; i < data.length; ) { (bool success, bytes memory result) = address(this).delegatecall(data[i]); if (!success) { assembly ("memory-safe") { revert(add(result, 32), mload(result)) } } results[i] = result; unchecked { ++i; } } }

Exploit Scenario

Alice, a member of the Panoptic team, adds a new functionality to the core protocol that adjusts users’ balances according to the msg.value. Eve, an attacker, uses the multicall functionality to increase her ETH balance without actually sending funds from her account, thereby stealing funds from the system.

Tools Used

Manual Review .

Recommended Mitigation Steps

Short term, document the risks associated with the use of msg.value and ensure that all developers are aware of this potential attack vector. Long term, detail the security implications of all functions in both the documentation and the code to ensure that potential attack vectors do not become exploitable when code is refactored or added.

References

● ”Two Rights Might Make a Wrong,” Paradigm ● https://solodit.xyz/issues/use-of-delegatecall-in-a-payable-function-inside-a-loop-trailofbits-yield-v2-pdf

Assessed type

call/delegatecall

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Insufficient proof