Design won't allow bridges with deterministic addresses

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L102-L106

Vulnerability details

The current design of the bridge doesn't allow for deterministic addresses for bridge contracts, since it will cause a conflict with nonce validation in the destination bridge.

Impact

Message replay is protected by using a nonce. The source bridge uses a monotonic increasing nonce that is sent in every bridged message, and the destination bridges checks if this nonce has been already executed:

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L102-L106

102:     if (isSpentNonce[chainToApprovedSender[srcChain]][nonce]) {
103:       revert NonceSpent();
104:     }
105: 
106:     isSpentNonce[chainToApprovedSender[srcChain]][nonce] = true;

While the isSpentNonce is correctly scoped to the address of the source bridge in the source chain (chainToApprovedSender[srcChain]), it doesn't consider the case that this address can be the same for different chains in both a planned or accidental scenarios.

Source bridge contracts may have the same address in different networks if they are deployed deterministically in a planned way (using CREATE2), or also in an accidental way as part of a script that reproduces the same deployment steps from a unique deployer account (using CREATE from the same address and with the same nonce).

If this is the case, messages from different source chains coming to the same destination chain will conflict with the nonce having the same key in the isSpentNonce mapping, leading to failures while executing the bridged messages.

Proof of Concept

Source bridge addresses are the same in source chain 1 (SC1) and source chain 2 (SC2).

1. User A bridges tokens from SC1 to destination chain (DC) using any nonce value N1.
2. Destination bridge in DC marks the nonce as consumed isSpentNonce[chainToApprovedSender[SC1]][N1] = true.
3. Later on, user B bridges tokens from SC2 to DC and encounters with nonce value N1.
4. Destination bridge in DC checks if nonce N1 is already spent. Since both addresses are the same chainToApprovedSender[SC1] == chainToApprovedSender[SC2], which means that isSpentNonce[chainToApprovedSender[SC2]][N1] == true, and the message is rejected.

Recommended Mitigation Steps

Scope the isSpentNonce mapping to include also the source chain so that nonces won't conflict if coming from the same source bridge address.

mapping(string => mapping(bytes32 => mapping(uint256 => bool))) public isSpentNonce;

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #406

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird marked the issue as not a duplicate

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)