ethereum-optimism / specs

OP Stack Specifications
https://specs.optimism.io
Creative Commons Zero v1.0 Universal
86 stars 80 forks source link

(Spec) Bug Report: Cyclic Attack Using Interop Message Passing #207

Open quintuskilbourn opened 4 months ago

quintuskilbourn commented 4 months ago

Looking at the current interop spec, I believe there may be a security issue when initiating and executing messages are executed at the same timestamp. If I'm right, the problem arises from logical precedence being lost when messages are passed.

Perhaps this is supposed to be addressed at a higher level of abstraction that I'm not anticipating. Apologies if that's the case - I am but a researchoor.

Example

Consider two blocks produced at the same timestamp by a malicious sequencer. Block A containing transaction T_a and block B containing transaction T_b.

Abstractly the attack runs like this:

The transactions would not be valid if executed sequentially, but they are when done on different chains in the superchain.

A more concrete attack:

On chain A there is a contract which allows anyone holding an NFT in collection C to mint another NFT in collection C, otherwise they can be bought for 10 Eth. Assume that no NFTs have been minted in the collection yet.

T_a is valid since it depends on m_b which is emitted on B and similarly T_b is valid since it depends only on m_a being emitted which it is - i.e. the attacker can bridge an NFT from B to A because it received the NFT via m_a.

Before the attack, the attacker didn't own an NFT and after the attack they do despite them not paying. Without the message passing protocol this wouldn't have been possible even with loans since no NFT existed before.

I realise this specific attack can be defended against at the smart contract level, but I thought I'd raise it since, from my understanding, this is a new problem opened up by synchronous message passing and might apply quite widely.

SkandaBhat commented 3 months ago

Initiating Messages are Events. You would call an executing message on the destination chain with an Identifier pointing to the Event or log in the source chain.

Please correct me if I did not understand the attack, I have been going through the spec as well. It would work in this way:

Assume Chain B has Chain A in the dependency set, and Chain A has Chain B in the dependency set. Assume these chains trust each other, and consider pre-confirmations to be safe, allowing sub-second interop among both chains with minimal latency.

  1. On block X on chain A, there is an NFT collection that lets you mint another NFT if you already have the NFT.
    function mint() public payable {
    if (IERC721(collectionAddress).balanceOf(msg.sender) > 0) {
        // User owns an NFT in collection C
        _mintNFT(msg.sender);
    } else if (msg.value >= 10 ether) {
        // User pays 10 ETH
        _mintNFT(msg.sender);
    } else {
        // Revert transaction with error
        revert("Must hold NFT from collection C or pay 10 ETH");
    }
    }
  2. On block X + 1, Alice mints an NFT sends the NFT to their wallet on Chain B. They send a "transferNFT" transaction on Chain A, that locks the NFT in the contract, and emits an event TransferNFT.

    function transferNFT(address to, uint256 tokenId) public {
    // Ensure the caller is the owner of the NFT
    require(ownerOf(tokenId) == msg.sender, "Caller is not the owner");
    
    // Transfer the NFT
    _transfer(msg.sender, to, tokenId);
    
    // Emit the TransferNFT event
    emit TransferNFT(msg.sender, to, tokenId);
    }
    1. Now we can call the CrossL2Inbox with the Identifier, and specify a target address on Chain B, that will create some sort of wrapped or bridged NFT with the same token ID and send it to Alice's wallet on Chain B. It emits an event MintBridgedNFT on block X + 1.
      
      // Define the BridgedNFT contract
      contract BridgedNFT is ERC721 {

    // Define the MintBridgedNFT event event MintBridgedNFT(address indexed to, uint256 indexed tokenId);

    // Function to mint bridged NFT function mintBridgedNFT(address to, uint256 tokenId) public { // Mint the bridged NFT _safeMint(to, tokenId); // Emit the MintBridgedNFT event emit MintBridgedNFT(to, tokenId); } }

Not sure I am seeing how we could use MintBridgedNFT emitted on Chain B to mint more free NFTs on Chain A. Could you clarify?

Also, cyclic dependencies are expected as Chain A must be able to communicate with Chain B and vice versa all the time, and should be possible to resolve if we look at them as a directed graph with vertices as blocks and edges as dependencies.

protolambda commented 3 months ago

Correct, we need to enforce a relative message ordering to support intra-block (same timestamp) messaging. Otherwise the initiating message can be after the executing message, and break core assumptions.

Intra-block messaging is an active area of research, and can be trivially disabled by tweaking the timestamp invariant in the smart-contract. Still, this is a good feature, so we are looking for ways to enforce the intra-block ordering elegantly.

The not-so-elegant solution would be to enforce a total ordering by event-log index, but when you consider cross-L2 intra-block messaging, this creates an undesired amount of incompatible cross-L2 messages. Ideally we use some form of relative ordering, to allow messages to interact with eachother, while keeping all dependencies in a form of a DAG (i.e. no cycles between messages, but possible still back and forth interactions between two blocks).

Note that this ordering can be enforced outside of the EVM; just like the dependency correctness is validated by the verifier, the ordering can also be checked.