sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

yixxas - Missing `payable` in `execute()` #385

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

yixxas

medium

Missing payable in execute()

Summary

A SubAccount can be used to execute a transaction. This includes transactions that has msg.value. Missing payable prevents users from executing transactions that requires ether.

Vulnerability Detail

execute() sends ether value to the to address. However, due to a missing payable, the function cannot receive ether. Contract also lacks the receive() function hence it cannot hold ether itself.

function execute(address to, bytes calldata data, uint256 value) external onlyOwner returns (bytes memory){
    require(to != address(0));
    (bool success, bytes memory returnData) = to.call{value: value}(data);
    if (!success) {
        assembly {
            let ptr := mload(0x40)
            let size := returndatasize()
            returndatacopy(ptr, 0, size)
            revert(ptr, size)
        }
    }
    emit ExecuteTransaction(owner, address(this), to, data, value);
    return returnData;
}

Impact

SubAccount cannot function as intended.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/subaccount/Subaccount.sol#L45-L58

Tool used

Manual Review

Recommendation

Consider adding payable modifier to execute()

Duplicate of #111