code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

The function `multiexcall` lacks aggregated value validation #237

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/factory/CashFactory.sol#L129

Vulnerability details

Impact

Gas token might get stuck/forgot/unable to retrieve from the contract.

Proof of Concept

Currently, the multiexcall allows the guardian to provide an arbitrary amount of msg.value and sends an arbitrary amount of value during each call within the loop. This might result in a mismatch of msg.value and aggregated value which can result in stuck funds in the worst case.

Through luck in misfortune, the excess gas tokens can be retrieved by simply providing zero msg.value during the function call but providing a matching value as calldata. However, this is clearly not the intended business logic and can result in a loss of funds if the guardian is unaware of this edge-case.

Tools Used

VSCode

Recommended Mitigation Steps

Consider aggregating the value (implement a totalValue variable and increase it) during each loop and execute a matching check with msg.value at the end of the function in order to ensure no excessive gas tokens are being sent to the contract.

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid