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

0 stars 2 forks source link

`Caller.callBatched` doesn't enforce `msg.value` is equal to sum of call values #290

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Caller.sol#L198

Vulnerability details

For each call in callBatched, we pass a value to be sent along with the call:

for (uint256 i = 0; i < calls.length; i++) {
    Call memory call = calls[i];
    returnData[i] = _call(sender, call.to, call.data, call.value);
}

The sum of the values of the calls should be equal to msg.value. If msg.value is insufficient, the batch will revert. If msg.value is greater than the sum of the values of the calls, then excess ETH will be left in the contract, which anyone can take by using callBatched with a call which sends the ETH to their address with msg.value == 0.

GalloDaSballo commented 1 year ago

I believe this shows appropriate nuance, but because the loss is self-inflicted and funds are technically recoverable, I think this is QA Low

xmxanuel commented 1 year ago

Known Issue. We don't think it is a real problem.

CodeSandwich commented 1 year ago

[dispute validity] It's an intended, well documented behavior that must be taken into account by the users. Lack of checks of spent Ether allows running more nuanced flows, e.g. recursive calls or getting Ether from 3rd party contracts.

c4-sponsor commented 1 year ago

CodeSandwich marked the issue as sponsor disputed

GalloDaSballo commented 1 year ago

Because of the comment: https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Caller.sol#L188-L189

I agree with the Sponsor

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid