Merkle implementation does not prevent replication of transactions across chains

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe37fb55a31f8d87b0655e71114e2825ce9b102b2694c790becc5002ec8784262 Severity: high

Description: Description\ - SUMMARY -

As stated in the Discord Q&A: "The Metrom contract will be deployed on multiple chains according to demand (if a dex strictly requires the Metrom campaigns to live on chain A, we'll deploy an instance there too)."

However, the existing implementation of merkle tree proofs fails to consider this aspect, thus enabling the replication of transactions across chains.

- DETAILS -

Within the claimRewards() function, the _processRewardClaim() function is called invoking MerkleProof.verifyCalldata() as shown in the code below.

• claimRewards()

  function claimRewards(ClaimRewardBundle[] calldata _bundles) external override {
      for (uint256 _i; _i < _bundles.length; _i++) {
          ClaimRewardBundle calldata _bundle = _bundles[_i];
      @>  uint256 _claimedAmount = _processRewardClaim(_getExistingCampaign(_bundle.campaignId), _bundle, msg.sender);
          emit ClaimReward(_bundle.campaignId, _bundle.token, _claimedAmount, _bundle.receiver);
      }
  }

• _processRewardClaim()

  function _processRewardClaim(Campaign storage campaign, ClaimRewardBundle calldata _bundle, address _claimOwner)
      internal
      returns (uint256)
  {
      if (_bundle.receiver == address(0)) revert InvalidReceiver();
      if (_bundle.token == address(0)) revert InvalidToken();
      if (_bundle.amount == 0) revert InvalidAmount();
  
      bytes32 _leaf = keccak256(bytes.concat(keccak256(abi.encode(_claimOwner, _bundle.token, _bundle.amount))));
  @>  if (!MerkleProof.verifyCalldata(_bundle.proof, campaign.root, _leaf)) revert InvalidProof(); 
  
      Reward storage reward = campaign.reward[_bundle.token];
      uint256 _claimAmount = _bundle.amount - reward.claimed[_claimOwner];
      if (_claimAmount == 0) revert ZeroAmount();
      if (_claimAmount > reward.unclaimed) revert InvalidAmount();
  
      reward.claimed[_claimOwner] += _claimAmount;
      reward.unclaimed -= _claimAmount;
  
      IERC20(_bundle.token).safeTransfer(_bundle.receiver, _claimAmount);
  
      return _claimAmount;
  }

  In _processRewardClaim() function, the line if (!MerkleProof.verifyCalldata(_bundle.proof, campaign.root, _leaf)) revert InvalidProof() can be bypassed on different chains by exploiting the deterministic nature of smart contracts (and EVM in general). If an attacker creates a successful transaction in Ethereum Mainnet, he/she can copy this and execute it on another EVM compatible chain.

Attack Scenario\

1. The attacker executes a transaction successfully on the Ethereum Mainnet by calling claimRewards() and satisfying all the arguments required in the _bundles parameter.
2. At this point, the core components of the exploit, namely the proof and root, have already been revealed.
3. On a different EVM chain where the contract is deployed, the attacker replicates the transaction by calling the claimRewards() function with identical arguments used in the successful transaction during Step 1.
4. Now, the attacker can acquire additional rewards on every chain where the contract is deployed.

- IMPACT -

Given that rewards can be replicated for each attacker across multiple chains, the protocol faces the risk of losing funds.

Attachments

1. Proof of Concept (PoC) File n/a

2. Revised Code File (Optional) n/a

[mcgrathcoutinho]

@luzzif This issue assumes there are two similar campaigns by the owner running on two different chains. In this case, if the user is able to claim rewards on both chain A and chain B, that means it is intentional for the user to earn rewards on both chains since the root is set by the updater. If the root on chain B is different, the user cannot claim the rewards. I do not think this is an issue since at the end it is the updater setting the roots across chains after seeing who is eligible to earn rewards and who is not.

[luzzif]

You're right on this one too actually. If the same root is on multiple chains, assuming the updater does its job correctly, it's only right that the same claim can be processed on both chains without any apparent issue.

[0xgreywolf]

Hi @luzzif and @mcgrathcoutinho.

The possibility of the transaction replay still exists and lingers. If a malicious campaign owner uses it to his/her advantage by creating two identical campaigns, then uses his/her own wallet (another wallet) and does the replay transaction himself/herself which is very feasible because he/she will basically replicate his/her rewards.

[luzzif]

But this wouldn't make much sense. The attacker would need to create a second campaign, putting rewards on the line, to only get a portion of them back by replaying a transaction/claim?

[0xgreywolf]

@luzzif Got it. You're right. I misinterpreted the purpose of the campaign itself. Thanks for explaining.