Submitted by 0xAlix2
Users can create a bridge on L1 that points to L2 chains, which allows them to transfer funds between the 2 chains. When creating a bridge an incremental ID is generated for that bridge, and saved. To deposit coins to an L2, users have to call InitiateTokenDeposit on the L1.
https://github.com/initia-labs/OPinit/blob/7da22a78f367cb25960ed9c536500c2754bd5d06/x/ophost/keeper/msg_server.go#L222-L274.
This generates an L1 sequence:
This sequence gets attached to the L2 message, when FinalizeTokenDeposit gets called, and gets validated on the L2, to ensure that messages are in order.
https://github.com/initia-labs/OPinit/blob/7da22a78f367cb25960ed9c536500c2754bd5d06/x/opchild/keeper/msg_server.go#L372-L487
This is all good for now. However, the issue is that the InitiateTokenDeposit function doesnt check if the provided input bridge ID is created before processing the message.
This allows a malicious user to initial a deposit on a non-existent bridge ID, which increments the l1Sequence to 2, the deposit sequence will be 1. This message wont be processed and sent to the L2, because simply the bridge/L2 doesnt exist.
Later, when a L2 and a bridge are created for that bridge ID (remember its incremental), any deposits initiated from the L1 will fail, because the sequence number will be > 1. On L2, finalizedL1Sequence will be 1, which forces the following validation to fail:
https://github.com/initia-labs/OPinit/blob/7da22a78f367cb25960ed9c536500c2754bd5d06/x/opchild/keeper/msg_server.go#L393-L395.
This DOSes the whole deposit functionality for any future bridges.
Add the following test in x/ophost/keeper/msg_server_test.go:
beer-1 (Initia) confirmed, but disagreed with severity and commented:
a_kalout (warden) commented:
3docSec (warden) commented:
a_kalout (warden) commented:
3docSec (warden) commented:
LSDan (judge) commented:
