A malicious actor can overtake the minting functionality in `aoTAP.sol` and easily make the broker related functions in `aoTAP.sol` unaccessible

[hats-bug-reporter[bot]]

Github username: @bauchibred Twitter username: bauchibred Submission hash (on-chain): 0x83e2e19b9ebaa378e931fc69c0f16db76789ab8da88c8b6f8c0714dab90a361c Severity: high

Description: Description

Take a look at https://github.com/hats-finance/Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad/blob/baddafc15616a5674e73c2f5a920b92bf9b21888/contracts/option-airdrop/aoTAP.sol#L130C1-L133C6

    function brokerClaim() external {
        if (broker != address(0)) revert OnlyOnce();
        broker = msg.sender;
    }

This function is used so the broker can come claim their status to be able to mint any amount of aoTAP, problem is that this implementation is unsafe and any one can just call this function before the expected broker to leave the contract useless as minting would be completely unacessible to the real broker, even worse since this broker status cannot be reclaimed by the owner, this means that the malicious actor can now mint infinite amount of aoTAP tokens.

Because only the broker is allowed to mint as seen here: https://github.com/hats-finance/Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad/blob/baddafc15616a5674e73c2f5a920b92bf9b21888/contracts/option-airdrop/aoTAP.sol#L102-L118

    function mint(address _to, uint128 _expiry, uint128 _discount, uint256 _amount, uint64 _phase)
        external
        nonReentrant
        returns (uint256 tokenId)
    {
        if (msg.sender != broker) revert OnlyBroker();

        tokenId = ++mintedAOTAP;
        AirdropTapOption storage option = options[tokenId];
        option.expiry = _expiry;
        option.discount = _discount;
        option.amount = _amount;
        option.phase = _phase;

        _safeMint(_to, tokenId);
        emit Mint(_to, tokenId, option);
    }

Attack Scenario A malicious griefer calls brokerClaim() before the expected broker calls it, making himself the broker. Recommendation Make the function an owner closed function instead, and then call this function with the owner address to set the respective broker address. Attachments

1. Proof of Concept (PoC) File

N/A

2. Revised Code File (Optional)

N/A

[0xRektora]

This was previously answered in our different audits. The brokerClaim() happens atomically within our deployments. Even if it weren't the case, we'd drop the compromised address for a new deployment.

[Bauchibred]

Hi @0xRektora, thanks for the comment, I don't know the judging rules on Hats as this is my first competition on here, so leaving this for them to decide, however this Atomicity was not indicated in the Docs neither was it commented in the code.

[0xRektora]

Thank you for participating we really appreciate it. However this would be a QA, which only medium and high are validated. Here is the same issue on one of our past audit.