AtomWallet::executeBatch differs from AtomWallet::execute because doesnt allow sending positive ETH amount while calling it breaking invariant/core code assumptions

Github username: -- Twitter username: -- Submission hash (on-chain): 0xb20d18541be769f9d40f65df1ecf7f582c2ff528769657bcccf704bb546f26b5 Severity: low

Description: Description\ Severity: Low due to breaking functionallity assumption could lead to unfulffiled state changes when using executeBatch as shown below

AtomWallet::executeBatch differs from AtomWallet::execute because doesnt allow sending ETH while calling it breaking invariant/core code assumptions.

AtomWallet::executeBatch is a function which goal is according to source code documentation to "execute a sequence (batch) of transactions" however it fails to fullfill expectation when sending positive (non zero) eth values where execute can fullfill this requirement.

To show the vulnerability suppose an user defines two txs txA and txB to send k ETH to two users userA and userB (for sample purposes, but the same applies for calling 2 contracts payable functions)

Then user could:

Perform two calls to AtomWallet::execute ie call AtomWallet::execute(txA) and txB.
The expected (and fullfilled) change is that k is added to userA and userB balance.

Call AtomWallet::executeBatch with arguments:
```
executeBatch([userA,userB],[txA,txB]) 
```
The expected (and NOT fullfilled) change is that after calling AtomWallet::executeBatch with the defined parameters (same txA and txB) k is added to userA and userB balance.
However this doesnt happen cause in executeBatch 0 eth value is hardcoded in inner call to _call function
```
function executeBatch(address[] calldata dest, bytes[] calldata func) external onlyOwnerOrEntryPoint {
    // ... snippet ...
    for (uint256 i = 0; i < dest.length; i++) {
        _call(dest[i], 0, func[i]);                 // <@= 0 ETH value is hardcoded here
    }
}
```
So this made imposible to executeBatch transactions where the eth value to send is positive (for example while sending eth to users or calling payable functions)
Observe this differs from AtomWallet::execute code where is not harcoded value
```
function execute(address dest, uint256 value, bytes calldata func) external onlyOwnerOrEntryPoint {
    _call(dest, value, func);   // <@ No hardcoded value
}
```
And AtomWallet::_call second argument defines how much eth to send to dest:
```
function _call(address target, uint256 value, bytes memory data) internal {
    (bool success, bytes memory result) = target.call{value: value}(data);  //<@ value defines eth to send
    // ... snippet 
}
```
So users could expect that calling executeBatch to payable functions (or sending eth to users) will perform state changes but this is not the case, breaking code expectations and assumptions

Attack Scenario\ As stated above the flaw in executeBatch could lead to users false expectations about executeBatch function while calling with positive ETH values and the expected but not fullfilled asociated to this calls

Proof of Concept

The PoC attached in file intuition_poc.sol shows the previous described case, user define two txs to send 1 eth each to user A and userB.

Using AtomWallet::execute(txA) and AtomWallet::execute(txB) the ETH balance for userA and userB is incremented

Next it used the same tx bytes to call executeBatch But while batch calling using AtomWallet::executeBatch([userA,userB],[txA,txB]) the balance is not incremented but executeBatch return success showing the uncapability of executeBatch to call with positive ETH values

Add PoC intuition_poc.sol content as a new test case to test/unit/EthMultiVault/Helpers.t.sol and execute test with verbose mode:

forge test -vvv --mt testAtomWalletExecuteAndExecuteBatchDiffResults

Observe balance is incremented when using execute(txA) and execute(txB) but not while batching tx with executeBatch

Suggested Fix

Remove the hardcoded 0 value in AtomWallet::executeBatch _call call and set a new values array in parameter as in intuition_fix.sol attached file and as shown below:

    function executeBatchFix(address[] calldata dest, uint256[] calldata values, bytes[] calldata func) external onlyOwnerOrEntryPoint {
        if (dest.length != func.length && func.length != values.length) {
            revert Errors.AtomWallet_WrongArrayLengths();
        }

        for (uint256 i = 0; i < dest.length; i++) {
            _call(dest[i], values[i], func[i]);
        }
    }

Attachments

Proof of Concept (PoC) File
Revised Code File (Optional)

Files:

intuition_fix.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmSCGum8ZpSiXckBDB2DKNKgddjNmTJzEvTbVMCJxrbmXy)
intuition_poc.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmV7TkXy4unqRsncB2yn6SwVbpYWc15ed1Eph1rW43ondg)

hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

AtomWallet::executeBatch differs from AtomWallet::execute because doesnt allow sending positive ETH amount while calling it breaking invariant/core code assumptions #61

Proof of Concept

Suggested Fix