function brokerClaim is vulnerable to frontrunning

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/oTAP.sol?plain=1#L126 https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/option-airdrop/aoTAP.sol?plain=1#L139

Vulnerability details

Impact

The attacker can execute the function before the deployer due to front-running, after which the broker variable will become equal to the address of the attacker, and attacker will gain control over all the functions of the smart contract with the onlyBroker modifier.

Proof of Concept

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/oTAP.sol?plain=1#L126

  function brokerClaim() external {
    require(broker == address(0), "OTAP: only once");
    broker = msg.sender;
  }

Tools Used

manual

Recommended Mitigation Steps

It is necessary to move the initialization of the broker variable to the constructor or ensuring atomic creation+deployment with script or factory contract.

Assessed type

MEV

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xRektora marked the issue as disagree with severity

[0xRektora]

informational, has to be called 1 time, at the time of protocol deployment. If not claimed by actual owner, contract can be redeployed.

[c4-sponsor]

0xRektora marked the issue as sponsor confirmed

[dmvt]

I agree with the sponsor. The full risk is gas cost of redeploying. No one can mint beside the broker, so protocol funds cannot be lost here. Downgrading to QA for medium risk reports, overinflated for high risk.

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-b