PalmeraModule.sol#execTransactionOnBehalf() - Any ETH send alongside the function call will be lost

[hats-bug-reporter[bot]]

Github username: -- Twitter username: @EgisSec Submission hash (on-chain): 0x07821447d9ab0b24a42806fb66b54d9afc8155669f554f69f9f9c5a0a429d59d Severity: high

Description: Description\ The function is payable and has a value argument.

The value argument is then passed to execTransactionFromModule.

Notice that the function isn't payable so it can't receive ETH.

/// @dev Allows a Module to execute a Safe transaction without any further confirmations.
    /// @param to Destination address of module transaction.
    /// @param value Ether value of module transaction.
    /// @param data Data payload of module transaction.
    /// @param operation Operation type of module transaction.
    function execTransactionFromModule(
        address to,
        uint256 value,
        bytes memory data,
        Enum.Operation operation
    ) public virtual returns (bool success) {
        // Only whitelisted modules are allowed.
        require(msg.sender != SENTINEL_MODULES && modules[msg.sender] != address(0), "GS104");
        // Execute transaction without further confirmations.
        success = execute(to, value, data, operation, type(uint256).max);
        if (success) emit ExecutionFromModuleSuccess(msg.sender);
        else emit ExecutionFromModuleFailure(msg.sender);
    }

Currently any ETH send alongisde execTransactionOnBehalf will be lost, as it doesn't forward it to the Safe.

Attack Scenario\

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Gnosis Safes inherit EtherPaymentFallback so if you want to send ETH to the Safe and the immediatly use it for execTransactionFromModule, then add the following line.

(bool success, ) = payable(targetSafe).call{value: msg.value}("");
        require(success);

[alfredolopez80]

Non-issue @0xRizwan becuase are not clear about the functionality of Safe Module and the execTransactionFromModule, when any module call this method, this execution occur into the safe itself, not from the module, in the end the money is not into the module of Palmera, the ETH are into the targetSafe indicate in the args of execTransactionOnBehalf, so not need to transfer anything,

if you wanna verify that check pls the Complete Unit-Test about this method: https://github.com/hats-finance/Palmera-0x5fee7541ddcd51ba9f4af606f87b2c42eea655be/blob/main/test/ExecTransactionOnBehalf.t.sol

[0xRizwan]

Safe's execTransactionFromModule() by default is non-payable which means ethers are not expected to be sent with function call. I think, the issue submitter could produce more details, if there is something wrong in understanding this issue. Would leave the final decision to @alfredolopez80