Ondo Finance - SpicyMeatball's results

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L79 https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L108-L109 https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L137

Vulnerability details

Impact

In the DestinationBridge contract we can set multiple source chain bridges to receive payloads from

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

The problem is that we can have identical payloads from different chains, for example payload

bytes memory payload = abi.encode("1.0", alice, 10000, 1)

can be sent from Arbitrum and Optimism networks, and when executed will produce the same hash. This will result in inability to register payload from Optimism because bridge will attempt to approve the same hash twice.

Proof of Concept

Test case for forge-tests/bridges/DestinationBridge.t.sol

  function test_TxHashCollision() public {
    // send message from Arbitrum chain
    bytes memory payload = abi.encode(VERSION, alice, 100e18, 1);
    sendMessage(payload);
    // add new source chain
    string memory srcAddress = "0x9019295cA2dCdC4BCFf323b562e87FC922e34C32";
    string memory srcChain = "optimism";
    vm.prank(guardian);
    destinationBridge.addChainSupport(srcChain, srcAddress);
    assertEq(
      destinationBridge.chainToApprovedSender("optimism"),
      keccak256(abi.encode(srcAddress))
    );
    // send message from Optimism chain
    bytes32 cmdId = bytes32(
      0x9991faa1f435675159ffae64b66d7ecfdb55c29755869a18db8497b4392347e0
    );

    approve_message(
      cmdId,
      srcChain,
      srcAddress,
      address(destinationBridge),
      keccak256(payload)
    );
    // revert as bridge attempts to approve the same hash twice
    vm.expectRevert(DestinationBridge.AlreadyApproved.selector);
    destinationBridge.execute(cmdId, srcChain, srcAddress, payload);
  }

Tools Used

Forge

Recommended Mitigation Steps

Add block.chainid parameter into the payload

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L79

Assessed type

Context