[audit] MultiBatchHeader signature does not include sending network or leaf index

Severity: Medium Difficulty: Medium Type: Data Validation Target: crates/pessimistic-proof/src/{local_state.rs,imported_bridge_exit.rs,bridge_exit.rs}

Description

The signature included in each MultiBatchHeader covers the new local exit root for the relevant network, as well as a hash of the bridge exits being imported. However, the imported bridge exit hash does not include the sending network ID or the leaf index. With one signature, an entity who controls the multi-batch being submitted can choose exactly which exits are nullified, so long as those exits have the same token type, destination address, and amount.

Note that while ImportedBridgeExit includes sending_network and leaf_index, the hash() function just returns self.bridge_exit.hash(): https://github.com/agglayer/agglayer/blob/92159de06d6ea82c7cf7beeb13f53bb2a401a0ff/crates/pessimistic-proof/src/imported_bridge_exit.rs#L11-L31 https://github.com/agglayer/agglayer/blob/92159de06d6ea82c7cf7beeb13f53bb2a401a0ff/crates/pessimistic-proof/src/imported_bridge_exit.rs#L60-L62

If no local exits are added, and thus the initial and final local exit roots are equal, an attacker could use a single signed MultiBatchHeader to complete additional transfers without additional authorization.

This does not allow an attacker to create additional total supply, but it may allow an attacker to create transfers that would otherwise be rejected.

Exploit Scenario

Alice exploits XChain and attempts to bridge exactly 2 ETH to BobChain. Bob, who controls the BobChain signing key for its AggLayer bridge, notices that this 2 ETH transfer is with stolen funds. He chooses to block the transfer, and simply ignores the exit root created by Alice. At a later time, Alice observes a multi-batch that Bob has signed which imports a 2 ETH from YChain without adding any local exits. She re-uses this multi-batch to complete the earlier transfer, and successfully steals funds from XChain.

Recommendations

Short term, add the sending network ID and leaf index to the signature. Long term, ensure that signatures cover all fields which can change the effect of an operation.

Hey! good finding, and i agree that we should make the signature to identify in a unique way what's inside the proof.

That being said i think the exploit scenario described is not possible, or this issue is not related on that happening.

Take in mind that the most typical case is that a network has " positive balance" , since there will be tokens/ether from other chains circulating in this chain (e.g ether)
So the only way to stop this kind of issues is:
- a. You are the operator on xCHAIN and basically censor Alice
- b. You are the operator on BoBChain and censor the claim to that bridge_exit
In any way affects from "where" the accountability of this ether comes from. Take in mind that even this scenario is possible: I "steal" 2 ethers, and then i want to withdraw them, but operator does not want to "account that bridge_exit". I send 2 ethers ( from a non-linked account), giving positive balance to the network, and then withdraw the 2 malicious ether. Afterwards i can just claim to the agglayer that i have 2 "innocent" ethers that i'm not able to bridge back.
Also in order to perform this kind of attacks you must be able to compute the proof for a network, and currently we are on a centralized scenario ( take in mind that otherwise we need to add more public inputs, which is more expensive but let the option to decentralize the proof computation e.g all bridge exits, which currently we though in future versions).

In any case we will sign something to be unique, for example, the nullifier root, or we could add the global index to the bridgeExitHash for example ^^

agglayer / agglayer