`Caller.callBatched`, `Caller.callSigned` and `Caller.callAs` do not track sent ETH

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Caller.sol#L144-L151 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Caller.sol#L164-L183 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Caller.sol#L193-L200

Vulnerability details

Description

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

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

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

Drips provide functionality for simplifying calls into the system via the Caller contract.

As a reference, the code for the batched call functionality is below:

function callBatched(Call[] memory calls) public payable returns (bytes[] memory returnData) {
    returnData = new bytes[](calls.length);
    address sender = _msgSender();
    for (uint256 i = 0; i < calls.length; i++) {
        Call memory call = calls[i];
        returnData[i] = _call(sender, call.to, call.data, call.value);
    }
}

This call assumes that the amount of ETH sent along with the call is exactly enough to cover all of the batched underlying calls. It does not perform any underage checks, nor does it refund overage.

The case of the user not sending enough ETH is generally harmless enough, just resulting in wasted gas. However, not refunding unused ETH represents a value leak.

Proof of Concept

Alice wants to use the batched call to perform actions on another contract, in order to have all of the actions execute in the same block. She's unsure of how much value the transactions will require, as the amounts estimated contain many decimal values, i.e., .438846780944 ETH. So she provides a best guess as to a value that will certainly cover the required sum.

Once she executes the transactions successfully, she finds that despite not using all of the ETH, the excess was not returned to her. It is sitting as a balance in the Caller contract. She could try to construct another call to retrieve the balance but risks being frontrun, or someone else (possibly in the same block as her first transaction) accidentally undersending ETH and using part of her balance in the contract.

Mitigation

Since there's no purpose to Caller maintaining an ETH balance, the contract could assume that any self-balance at the end of one of the calls is the leftover balance of the sender and remit that balance to msg.sender. It would also be best to compare the sum of all call values to msg.value at the outset to save the caller some gas if they didn't send enough ETH.

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

L

[c4-judge]

GalloDaSballo marked the issue as grade-c