Contract with a payable function, but without a withdrawal capacity

[hats-bug-reporter[bot]]

Github username: @makenzicloud Twitter username: -- Submission hash (on-chain): 0xb57bb9f698180ba738efcd1951eb0145f84de92892614cfe6034af107e2a70e3 Severity: high

Description: Description\ Thedeploy function in the TapiocaDeployer contract is marked as payable, allowing ether to be sent to the contract. However, there is no function to withdraw these funds, which could result in ether being locked in the contract.

  function deploy(uint256 amount, bytes32 salt, bytes memory bytecode, string memory contractName)
        external
        payable
        returns (address addr)
    {
        require(msg.value == amount, "Create2: insufficient balance");
        require(bytecode.length != 0, string.concat("Create2: bytecode length is zero for contract ", contractName));
        /// @solidity memory-safe-assembly
        assembly {
            addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt)
        }
        require(addr != address(0), string.concat("Create2: Failed on deploy for contract ", contractName));
    }

Attack Scenario\ An attacker could repeatedly send ether to the contract using the deploy function, causing funds to accumulate in the contract with no way to retrieve them. This could lead to a denial of service by locking ether within the contract indefinitely.

Attachments

1. Proof of Concept (PoC) File

pragma solidity ^0.8.0;

contract PoC {
    Deployer deployer;

    constructor(address _deployerAddress) {
        deployer = Deployer(_deployerAddress);
    }

    function testLockedFunds() external payable {
        deployer.deploy{value: msg.value}(msg.value, bytes32(0), hex"", "TestContract");
        // No way to withdraw funds after this point
    }
}

Mitigation example:

contract Deployer {
    address public owner;

    constructor() {
        owner = msg.sender;
    }

    modifier onlyOwner() {
        require(msg.sender == owner, "Not authorized");
        _;
    }

    function deploy(uint256 amount, bytes32 salt, bytes memory bytecode, string memory contractName)
        external
        payable
        returns (address addr)
    {
        require(msg.value == amount, "Create2: insufficient balance");
        require(bytecode.length != 0, string.concat("Create2: bytecode length is zero for contract ", contractName));

        /// @solidity memory-safe-assembly
        assembly {
            addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt)
        }

        require(addr != address(0), string.concat("Create2: Failed on deploy for contract ", contractName));
    }

    function withdraw() external onlyOwner {
        payable(owner).transfer(address(this).balance);
    }
}

Recommendations Remove payable Attribute:

• If the contract does not need to hold Ether, remove the payable keyword from the deploy function. Add Withdraw Function:
• Implement a function to allow Ether withdrawal.

2. Revised Code File (Optional)

[0xRektora]

This is out of scope and not a valid finding.