Ondo Finance - bowtiedvirus's results

Lines of code

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

Vulnerability details

Effected Contract: DestinationBridge.sol

Summary

The hashed payload sent to _execute in DestinationBridge can result in a collision that overwrites a previously saved but not yet approved transaction if two transactions are sent from two seperate chains, but with the same payload details.

In this report, I will detail the root cause of the issue, which a user could accidentally encounter, then detail a possible front-running grief exploit utilizing this vulnerability.

Vulnerability Details

The payload in DestinationBridge uses information that is not mutually exclusive between chains, resulting in a possible hash collision that silenty overwrites incomplete transactions, resulting in permanently lost funds. Nonce is a particularly tricky part of the payload, since two distinct chains could have the same nonce in their Source Bridge.

Reproduction Steps

1. SourceBridge SA and SB are deployed on chain A and chain B.
2. DestinationBridge DC is deployed on chain C, and a threshold is set in which any mint amount above 0 needs at least 2 approvers.
3. In the current position in the block, the nonce for SA is equal to the nonce of SB referred to herein as overlapping_nonce.
4. Alice calls burnAndCallAxelar(100 ether, address(DC)) on SA and SB at the same time.
5. The tx from SA reaches DC, and the tx is created with key keccak256(VERSION, alice, 100 ether, overlapping_nonce), but awaiting another approval before minting the 100 tokens.
6. The tx from SB reaches DC and the hash is the same as the tx from SA, silently overwriting the tx from SA.

Proof of Concept Test

// DestinationBridge.t.sol
function test_Receiver_can_be_overwritten_if_same_payload_is_sent_from_another_chain() public {
    vm.label(alice, "alice");

    bytes memory payload = abi.encode(VERSION, alice, 100e18, 1);
    bytes32 txnHash = keccak256(payload);

    bytes memory payloadOtherChain = abi.encode(VERSION, alice, 100e18, 1);
    bytes32 txnHashOtherChain = keccak256(payloadOtherChain);
    
    // The contents of the payload are the same. Therefore the hashes are the same.
    assertEq(
      txnHashOtherChain,
      txnHash
    );

    bytes32 cmdIdPolygon = bytes32(
      0x1234faa1f435675159ffae64b66d7ecfdb55c29755869a18db8497b43923adf9
    );
    string memory srcPolygonChain = "Polygon";
    string memory srcPolygonAddress = "0xPolygonSourceChainAddressPolygonSourcePo";

    approve_message(
      cmdIdPolygon,
      srcPolygonChain,
      srcPolygonAddress,
      address(destinationBridge),
      keccak256(payload)
    );

    vm.expectEmit(true, true, true, true);
    emit ChainIdSupported(
      "Polygon",
      "0xPolygonSourceChainAddressPolygonSourcePo"
    );

    initializeArrayValues();

    vm.startPrank(guardian);
    destinationBridge.addChainSupport(srcPolygonChain, srcPolygonAddress);
    destinationBridge.setThresholds(srcPolygonChain, amounts, numOfApprovers);
    vm.stopPrank();

    // Send msg from Chain A.
    vm.prank(address(gateway_eth));
    destinationBridge.execute(cmdIdPolygon, srcPolygonChain, srcPolygonAddress, payload);

    // Send msg from Chain B, which overwrites the txn with the same payload details.
    vm.prank(address(gateway_eth));
    sendMessage(payloadOtherChain);

    vm.expectEmit(true, true, true, true);
    emit BridgeCompleted(alice, 100e18);
    vm.prank(ondoSigner0);
    destinationBridge.approve(keccak256(payloadOtherChain));

    // Oh no! We shouldn't be reverting, we need to approve the original tx!!!
    vm.expectRevert();
    vm.prank(ondoSigner0);
    destinationBridge.approve(keccak256(payload));

    // Should be 200e18, but since _execute overwrote our original tx, we lose those assets forever.
    assertEq(usdy.balanceOf(alice), 100e18);
  }

Explicit Griefing Exploit

1. Bob, a shadowy super coder that enjoys griefing others on the blockchain, could run a bot that detects that Alice has queued up multiple burnAndCallAxelar transactions on multiple blockchains, each with a nonce that are fairly close to each other, the same amount of tokens, and the same destination chain.
2. Bob front-runs the transaction(s) and calls burnAndCallAxelar himself multiple times to increment the nonce of the SourceBridges.
3. Once 2 or more SourceBridges have the same nonce, Bob rests.
4. When Alice's transaction is approved, she will only receive tokens from the last transaction to be executed.

Impact

A user can permanently lose tokens sent to a DestinationBridge.

Tools Used

Manual Review

Recommendations

1. Add a chain-specific value to the hashed value in DestinationBridge.

// DestinationBridge.sol
110| bytes32 txnHash = keccak256(abi.encodePacked(payload, srcChain));

This information is already available in the _execute() transaction, but tests utilizing approve() will need to be modified to accomodate the modified hash.

Assessed type

Math