ExecutorPlugin missing payable when execute the transaction

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/ExecutorPlugin.sol#L68 https://github.com/code-423n4/2023-10-brahma/blob/dd0b41031b199a0aa214e50758943712f9f574a0/contracts/src/core/ExecutorPlugin.sol#L90

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

executeTransaction in ExecutorPlugin is meant to execute transaction

but in _executeTxnAsModule

function _executeTxnAsModule(address _account, Types.Executable memory _executable)
        internal
        returns (bytes memory)
    {
        (bool success, bytes memory txnResult) = IGnosisSafe(_account).execTransactionFromModuleReturnData(
            _executable.target,
            _executable.value,
            _executable.data,
            SafeHelper._parseOperationEnum(_executable.callType)
        );
        if (!success) revert ModuleExecutionFailed();
        return txnResult;
    }

the code does not forward ETH when execute the transaction,

executeTransaction does not have payable keywords as well, meaning the plugin is not capable of attaching ETH when execute the transaction

Tools Used

Manual Review

Recommended Mitigation Steps

add payable keywords in function executeTransaction and forward the ETH when execute the transaction

(bool success, bytes memory txnResult) = IGnosisSafe(_account){value: msg.value}.execTransactionFromModuleReturnData(
            _executable.target,
            _executable.value,
            _executable.data,

Assessed type

ETH-Transfer

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #363

[c4-pre-sort]

raymondfam marked the issue as not a duplicate

[c4-pre-sort]

raymondfam marked the issue as duplicate of #470

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid