fix(security): validate merkle root before broadcast and reset isVerified on permission revocation (#6716)

xxo1shine · web-flow · commit 4f41f26404b1 · 2026-05-09T12:13:59.000+08:00
diff --git a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
@@ -15,6 +15,8 @@
 
 package org.tron.core.capsule;
 
+import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED;
+
 import com.google.common.primitives.Longs;
 import com.google.protobuf.ByteString;
 import com.google.protobuf.CodedInputStream;
@@ -37,6 +39,7 @@
 import org.tron.common.utils.Time;
 import org.tron.core.capsule.utils.MerkleTree;
 import org.tron.core.config.Parameter.ChainConstant;
+import org.tron.core.exception.BadBlockException;
 import org.tron.core.exception.BadItemException;
 import org.tron.core.exception.ValidateSignatureException;
 import org.tron.core.store.AccountStore;
@@ -49,6 +52,7 @@
 public class BlockCapsule implements ProtoCapsule<Block> {
 
   public boolean generatedByMyself = false;
+  private volatile boolean merkleValidated = false;
   @Getter
   @Setter
   private TransactionRetCapsule result;
@@ -225,6 +229,19 @@ public Sha256Hash calcMerkleRoot() {
     return MerkleTree.getInstance().createTree(ids).getRoot().getHash();
   }
 
+  public void validateMerkleRoot() throws BadBlockException {
+    if (merkleValidated) {
+      return;
+    }
+    Sha256Hash actual = calcMerkleRoot();
+    if (!actual.equals(getMerkleRoot())) {
+      throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
+          String.format("merkle root mismatch for block %d: expected %s, actual %s",
+              getNum(), getMerkleRoot(), actual));
+    }
+    merkleValidated = true;
+  }
+
   public void setMerkleRoot() {
     BlockHeader.raw blockHeaderRaw =
         this.block.getBlockHeader().getRawData().toBuilder()
diff --git a/framework/src/main/java/org/tron/core/db/Manager.java b/framework/src/main/java/org/tron/core/db/Manager.java
@@ -1307,12 +1307,7 @@ public void pushBlock(final BlockCapsule block)
         try (PendingManager pm = new PendingManager(this)) {
 
           if (!block.generatedByMyself) {
-            if (!block.calcMerkleRoot().equals(block.getMerkleRoot())) {
-              logger.warn("Num: {}, the merkle root doesn't match, expect is {} , actual is {}.",
-                  block.getNum(), block.getMerkleRoot(), block.calcMerkleRoot());
-              throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
-                      String.format("The merkle hash is not validated for %d", block.getNum()));
-            }
+            block.validateMerkleRoot();
             consensus.receiveBlock(block);
           }
 
@@ -2130,6 +2125,13 @@ public void rePush(TransactionCapsule tx) {
       return;
     }
 
+    String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress());
+    synchronized (this) {
+      if (ownerAddressSet.contains(ownerAddress)) {
+        tx.setVerified(false);
+      }
+    }
+
     try {
       this.pushTransaction(tx);
     } catch (ValidateSignatureException | ContractValidateException | ContractExeException
diff --git a/framework/src/main/java/org/tron/core/net/TronNetDelegate.java b/framework/src/main/java/org/tron/core/net/TronNetDelegate.java
@@ -347,6 +347,11 @@ public boolean validBlock(BlockCapsule block) throws P2pException {
       throw new P2pException(TypeEnum.BAD_BLOCK,
               "time:" + time + ",block time:" + block.getTimeStamp());
     }
+    try {
+      block.validateMerkleRoot();
+    } catch (BadBlockException e) {
+      throw new P2pException(TypeEnum.BLOCK_MERKLE_ERROR, e.getMessage());
+    }
     validSignature(block);
     return witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress());
   }
diff --git a/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java b/framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java
@@ -19,6 +19,7 @@
 import org.tron.common.utils.Sha256Hash;
 import org.tron.core.Wallet;
 import org.tron.core.config.args.Args;
+import org.tron.core.exception.BadBlockException;
 import org.tron.core.exception.BadItemException;
 import org.tron.protos.Protocol.Transaction.Contract.ContractType;
 import org.tron.protos.contract.BalanceContract.TransferContract;
@@ -85,6 +86,43 @@ public void testCalcMerkleRoot() throws Exception {
     logger.info("Transaction[O] Merkle Root : {}", blockCapsule0.getMerkleRoot().toString());
   }
 
+  @Test
+  public void testValidateMerkleRoot() throws Exception {
+    // build a fresh local block so shared blockCapsule0 is not mutated
+    String parentHash = "9938a342238077182498b464ac0292229938a342238077182498b464ac029222";
+    BlockCapsule local = new BlockCapsule(1,
+        Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(parentHash))),
+        1234,
+        ByteString.copyFrom("1234567".getBytes()));
+
+    // valid block: setMerkleRoot then validate — should not throw
+    local.setMerkleRoot();
+    local.validateMerkleRoot(); // no exception
+
+    // flag is set — second call must be a no-op (no recomputation)
+    local.validateMerkleRoot(); // still no exception
+
+    // tamper with a transaction to break merkle
+    TransferContract transferContract = TransferContract.newBuilder()
+        .setAmount(999L)
+        .setOwnerAddress(ByteString.copyFrom("0x0000000000000000000".getBytes()))
+        .setToAddress(ByteString.copyFrom(ByteArray.fromHexString(
+            Wallet.getAddressPreFixString() + "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
+        .build();
+    local.addTransaction(
+        new TransactionCapsule(transferContract, ContractType.TransferContract));
+    // merkle root was set before adding the tx, so it is now stale/invalid
+
+    BlockCapsule tampered = new BlockCapsule(local.getInstance());
+    // tampered has no merkleValidated flag set
+    try {
+      tampered.validateMerkleRoot();
+      Assert.fail("Expected BadBlockException for merkle mismatch");
+    } catch (BadBlockException e) {
+      Assert.assertTrue(e.getMessage().contains("merkle"));
+    }
+  }
+
   /* @Test
   public void testAddTransaction() {
     TransactionCapsule transactionCapsule = new TransactionCapsule("123", 1L);
diff --git a/framework/src/test/java/org/tron/core/db/ManagerMockTest.java b/framework/src/test/java/org/tron/core/db/ManagerMockTest.java
@@ -380,7 +380,18 @@ public void testRePush() {
   @Test
   public void testRePush1() {
     Manager dbManager = spy(new Manager());
-    Protocol.Transaction transaction = Protocol.Transaction.newBuilder().build();
+    BalanceContract.TransferContract transferContract =
+        BalanceContract.TransferContract.newBuilder()
+            .setOwnerAddress(ByteString.copyFromUtf8("aaa"))
+            .setToAddress(ByteString.copyFromUtf8("bbb"))
+            .setAmount(1)
+            .build();
+    Protocol.Transaction transaction = Protocol.Transaction.newBuilder()
+        .setRawData(Protocol.Transaction.raw.newBuilder()
+            .addContract(Protocol.Transaction.Contract.newBuilder()
+                .setParameter(Any.pack(transferContract))
+                .setType(Protocol.Transaction.Contract.ContractType.TransferContract)))
+        .build();
     TransactionCapsule trx = new TransactionCapsule(transaction);
     TransactionStore transactionStoreMock = mock(TransactionStore.class);
 
diff --git a/framework/src/test/java/org/tron/core/db/ManagerTest.java b/framework/src/test/java/org/tron/core/db/ManagerTest.java
@@ -509,8 +509,8 @@ public void pushBlockInvalidMerkelRoot() {
     } catch (BadBlockException e) {
       Assert.assertTrue(e instanceof BadBlockException);
       Assert.assertTrue(e.getType().equals(CALC_MERKLE_ROOT_FAILED));
-      Assert.assertEquals("The merkle hash is not validated for "
-              + blockCapsule2.getNum(), e.getMessage());
+      Assert.assertTrue(e.getMessage().startsWith(
+          "merkle root mismatch for block " + blockCapsule2.getNum() + ":"));
     } catch (Exception e) {
       Assert.assertFalse(e instanceof Exception);
     }
@@ -1420,6 +1420,35 @@ public void testClearSolidityContractTriggerCache() throws Exception {
     }
   }
 
+  @Test
+  public void testRePushResetsVerifiedOnOwnerAddressSetHit() throws Exception {
+    TransferContract transferContract = TransferContract.newBuilder()
+        .setAmount(1L)
+        .setOwnerAddress(ByteString.copyFrom(
+            ByteArray.fromHexString(Wallet.getAddressPreFixString()
+                + "548794500882809695A8A687866E76D4271A1ABC")))
+        .setToAddress(ByteString.copyFrom(
+            ByteArray.fromHexString(Wallet.getAddressPreFixString()
+                + "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
+        .build();
+    TransactionCapsule tx = new TransactionCapsule(transferContract, ContractType.TransferContract);
+    tx.setVerified(true); // simulate mempool-cached state
+
+    String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress());
+
+    // Inject ownerAddress into ownerAddressSet via reflection
+    Set<String> ownerAddressSet =
+        (Set<String>) ReflectUtils.getFieldObject(dbManager, "ownerAddressSet");
+    ownerAddressSet.add(ownerAddress);
+
+    // rePush should reset isVerified to false before pushTransaction
+    dbManager.rePush(tx);
+
+    // After rePush, isVerified must be false
+    Boolean verified = (Boolean) ReflectUtils.getFieldObject(tx, "isVerified");
+    Assert.assertFalse(verified);
+  }
+
   @Test
   public void testGetCachedTransactionSize() throws Exception {
     BlockingQueue<TransactionCapsule> pushQ = new LinkedBlockingQueue<>();
diff --git a/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java b/framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java
@@ -2,16 +2,24 @@
 
 import static org.mockito.Mockito.mock;
 
+import com.google.protobuf.ByteString;
 import java.lang.reflect.Field;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.tron.common.TestConstants;
 import org.tron.common.parameter.CommonParameter;
+import org.tron.common.utils.ByteArray;
 import org.tron.common.utils.Sha256Hash;
 import org.tron.core.ChainBaseManager;
+import org.tron.core.Wallet;
 import org.tron.core.capsule.BlockCapsule;
+import org.tron.core.capsule.TransactionCapsule;
 import org.tron.core.config.args.Args;
+import org.tron.core.exception.P2pException;
+import org.tron.core.exception.P2pException.TypeEnum;
+import org.tron.protos.Protocol.Transaction.Contract.ContractType;
+import org.tron.protos.contract.BalanceContract.TransferContract;
 
 public class TronNetDelegateTest {
 
@@ -49,4 +57,37 @@ public void test() throws Exception {
 
     Assert.assertTrue(!tronNetDelegate.isBlockUnsolidified());
   }
+
+  @Test
+  public void testValidBlockMerkleRoot() throws Exception {
+    Args.setParam(new String[] {}, TestConstants.TEST_CONF);
+
+    String parentHash = "9938a342238077182498b464ac0292229938a342238077182498b464ac029222";
+    BlockCapsule block = new BlockCapsule(1,
+        Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(parentHash))),
+        System.currentTimeMillis(),
+        ByteString.copyFrom("witness".getBytes()));
+    block.setMerkleRoot();
+
+    // Add a transaction after setMerkleRoot, making the stored merkle root stale.
+    TransferContract transferContract = TransferContract.newBuilder()
+        .setAmount(1L)
+        .setOwnerAddress(ByteString.copyFrom("0x0000000000000000000".getBytes()))
+        .setToAddress(ByteString.copyFrom(ByteArray.fromHexString(
+            Wallet.getAddressPreFixString() + "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
+        .build();
+    block.addTransaction(
+        new TransactionCapsule(transferContract, ContractType.TransferContract));
+
+    // Wrap in a fresh BlockCapsule so the merkleValidated flag is reset.
+    BlockCapsule tampered = new BlockCapsule(block.getInstance());
+
+    TronNetDelegate tronNetDelegate = new TronNetDelegate();
+    try {
+      tronNetDelegate.validBlock(tampered);
+      Assert.fail("Expected P2pException for tampered merkle root");
+    } catch (P2pException e) {
+      Assert.assertEquals(TypeEnum.BLOCK_MERKLE_ERROR, e.getType());
+    }
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -347,6 +347,11 @@ public boolean validBlock(BlockCapsule block) throws P2pException {`
`347`	`347`	`throw new P2pException(TypeEnum.BAD_BLOCK,`
`348`	`348`	`"time:" + time + ",block time:" + block.getTimeStamp());`
`349`	`349`	`}`
	`350`	`+ try {`
	`351`	`+ block.validateMerkleRoot();`
	`352`	`+ } catch (BadBlockException e) {`
	`353`	`+ throw new P2pException(TypeEnum.BLOCK_MERKLE_ERROR, e.getMessage());`
	`354`	`+ }`
`350`	`355`	`validSignature(block);`
`351`	`356`	`return witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress());`
`352`	`357`	`}`