Open ryanschneider opened 1 month ago
Thanks for looking at the spec and giving this feedback @ryanschneider! You are definitely correct that due to the current definition it is possible to have two different events that are emitted result in the same serialized message.
Given the following definition:
msg := make([]byte, 0)
for _, topic := range log.Topics {
msg = append(msg, topic.Bytes()...)
}
msg = append(msg, log.Data...)
Given the following events:
event Foo(bytes32 indexed);
event Bar(bytes32, bytes32) anonymous;
You cannot distinguish the difference in the message between:
emit Foo(bytes32(0));
emit Bar(0x54b5e571557045df8a69633aeac8e206d92b03f48563fab6a430493d299de727, bytes32(0));
Under the current scheme, one would need to call back to the CrossL2Inbox
for auth, in particular origin() would let you know which contract emitted the event. We definitely want to enable interactions with untrusted contracts as much as possible. I am trying to wrap my head around how different the security model would be in practice if we had some sort of additional unique identifier within the message that would enable the events to be distinguished because an untrusted contract could still just emit the same event at the wrong place.
For example, emitting a Burn
event that can be used to Mint
on a remote chain, how do we know that its actually subtracting from the users balance? This leads me to believe there is some fundamental amount of distrust that you need to have when interacting with contracts on remote chains, which is not much different than interacting with untrusted contracts on a local chain.
I haven't thought of a concrete attack yet, but my reading of the Interop Spec is that the
Message Indentifer
doesn't fully account of every byte in the initiating message event. This reminded me of the recent Polygon Heimdal issue w/ event processing outlined here: https://www.asymmetric.re/blog/polygon-log-confusionThe TLDR of the issue is that if you don’t fully account for every byte in the event when processing it you leave yourself open to an attacker possibly being able to craft a malicious event that passes your parsing logic when it shouldn’t. In the polygon example this happened off-chain in the sequencer, but the same general type of flaw could exist in a contract, or in this case possibly the
CrossL2Inbox
predeploy.My concern is that the
Message Payload
is just a concatenation of the log fields, and that theMessage Identifer
doesn't include either a byte length or full hash or other exhaustive check of said payload. As such, I think its possible to craft an alternative message payload with say additional topics and pass that toexecuteMessage
possibly leading the emission of a "spoofed"ExecutingMessage
event for a different initiating message than intended.This would involve something like a malicious user emitting two very similar looking initiating messages, say one with
LOG1
and a second usingLOG2
that contains the same first topic (due to the current payload serialization format this would only work if the event only contained topics and no data).As I said I can't think of an actual attack with this yet, but wanted to highlight it as something that made my spider sense tingle a little when reading the spec.