hats-finance / Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad

4 stars 4 forks source link

Arbitrary `from` passed to `transferFrom`in Cross-chain Transactions (LayerZero) can cause loss of funds due to change in `msg.sender` #30

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xdae446b784acccf3b7cc39e5d44f49cfd6afba0226f93aa6b1f18a6eebc06f37 Severity: high

Description: Description\ Unchecked from when used in a cross-chain transaction with transferFrom can lead to loss of funds due to the change in msg.sender to the Cross-chain Executor Contract and tx.origin - Cross-chain Executor.

Attack Scenario\ Describe how the vulnerability can be exploited. Users Involved\

  1. Alice - a user
  2. A - a Cross-chain protocol like LayerZero
    • has an on-chain contract
    • has an off-chain executor
  3. Mallory - a hacker
  4. S - the source network
  5. D - the destination network
  6. X - an arbitrary network

The Attack

  1. Alice grants approval() to A's contract on D (this is standard for when you want to issue a transaction that moves funds from one network to another such as Orbiter or CowSwap or SquidRouter) or Mallory gets Alice to sign an approval(). In the second case Alice who is a user of the protocol will sign it because well she thinks she is giving access to the "exchange" to handle her fund transfers.
  2. Mallory can now use the same cross-chain protocol A and send a transaction from X to D. A's cross-chain executor now picks up the transaction and submits it on the destination network. Note that the tx.origin is now A's cross-chain executor. The transaction lands on A's contract which then routes the transaction to the destination as defined by Mallory. The msg.sender here is now A's contract.
  3. Given that transferFrom() accepts arbitrary from Mallory could have the destination as an ERC-20 transferFrom() which takes in msg.sender - A's cross-chain contract, to - Mallory's address, and amount. As Alice had approved the cross-chain contract to handle token transfers this exploit now results with Mallory stealing Alice's funds

Attachments

  1. Proof of Concept (PoC) File\ The PoC catches the changed tx.origin and msg.sender. The protocol uses LayerZero which does a simulation on the network and doesn't send a destination transaction if it detects a `transferFrom(). However the simulator can ONLY detect on-chain transactions, and should the attacker use another smart contract to offload the transaction off-chain through CoW or Uniswap X or Oribiter or 0x or any of the other cross-chain transaction protocols that accept a signed transaction to fill later, those protocols now have a valid chain of transactions with approvals that can execute them.

  2. Revised Code File (Optional)

    • Disable cross-chain protocol msg.sender or tx.origin from interacting with the protocol
    • Add a wrapper function around transferFrom() to disable them in the event of an exploit

Points of Attack

  1. TapiocaZ

    • contracts/tOFT/TOFT.sol#L102
    • contracts/tOFT/mTOFT.sol#L142
  2. TapToken

    • contracts/tokens/TapToken.sol#L177

All of them contain:\ return BaseTapiocaOmnichainEngine.transferFrom(from, to, value);

Which is part of LayerZero messaging through Omnichain

Files:

0xRektora commented 5 months ago

The PoC are completely different from our code.