Skip to content

Commit 193f361

Browse files
wbclaude
authored and
wb
committed
fix(security): validate merkle root before broadcast and reset isVerified on permission revocation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 980c707 commit 193f361

6 files changed

Lines changed: 110 additions & 9 deletions

File tree

‎chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.tron.common.utils.Time;
3838
import org.tron.core.capsule.utils.MerkleTree;
3939
import org.tron.core.config.Parameter.ChainConstant;
40+
import org.tron.core.exception.BadBlockException;
41+
import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED;
4042
import org.tron.core.exception.BadItemException;
4143
import org.tron.core.exception.ValidateSignatureException;
4244
import org.tron.core.store.AccountStore;
@@ -49,6 +51,7 @@
4951
public class BlockCapsule implements ProtoCapsule<Block> {
5052

5153
public boolean generatedByMyself = false;
54+
private volatile boolean merkleValidated = false;
5255
@Getter
5356
@Setter
5457
private TransactionRetCapsule result;
@@ -225,6 +228,19 @@ public Sha256Hash calcMerkleRoot() {
225228
return MerkleTree.getInstance().createTree(ids).getRoot().getHash();
226229
}
227230

231+
public void validateMerkleRoot() throws BadBlockException {
232+
if (merkleValidated) {
233+
return;
234+
}
235+
Sha256Hash actual = calcMerkleRoot();
236+
if (!actual.equals(getMerkleRoot())) {
237+
throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
238+
String.format("merkle root mismatch for block %d: expected %s, actual %s",
239+
getNum(), getMerkleRoot(), actual));
240+
}
241+
merkleValidated = true;
242+
}
243+
228244
public void setMerkleRoot() {
229245
BlockHeader.raw blockHeaderRaw =
230246
this.block.getBlockHeader().getRawData().toBuilder()

‎framework/src/main/java/org/tron/core/db/Manager.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,12 +1293,7 @@ public void pushBlock(final BlockCapsule block)
12931293
try (PendingManager pm = new PendingManager(this)) {
12941294

12951295
if (!block.generatedByMyself) {
1296-
if (!block.calcMerkleRoot().equals(block.getMerkleRoot())) {
1297-
logger.warn("Num: {}, the merkle root doesn't match, expect is {} , actual is {}.",
1298-
block.getNum(), block.getMerkleRoot(), block.calcMerkleRoot());
1299-
throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
1300-
String.format("The merkle hash is not validated for %d", block.getNum()));
1301-
}
1296+
block.validateMerkleRoot();
13021297
consensus.receiveBlock(block);
13031298
}
13041299

@@ -2106,6 +2101,13 @@ public void rePush(TransactionCapsule tx) {
21062101
return;
21072102
}
21082103

2104+
String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress());
2105+
synchronized (this) {
2106+
if (ownerAddressSet.contains(ownerAddress)) {
2107+
tx.setVerified(false);
2108+
}
2109+
}
2110+
21092111
try {
21102112
this.pushTransaction(tx);
21112113
} catch (ValidateSignatureException | ContractValidateException | ContractExeException

‎framework/src/main/java/org/tron/core/net/TronNetDelegate.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,11 @@ public boolean validBlock(BlockCapsule block) throws P2pException {
347347
throw new P2pException(TypeEnum.BAD_BLOCK,
348348
"time:" + time + ",block time:" + block.getTimeStamp());
349349
}
350+
try {
351+
block.validateMerkleRoot();
352+
} catch (BadBlockException e) {
353+
throw new P2pException(TypeEnum.BLOCK_MERKLE_ERROR, e.getMessage());
354+
}
350355
validSignature(block);
351356
return witnessScheduleStore.getActiveWitnesses().contains(block.getWitnessAddress());
352357
}

‎framework/src/test/java/org/tron/core/capsule/BlockCapsuleTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.tron.common.utils.Sha256Hash;
2020
import org.tron.core.Wallet;
2121
import org.tron.core.config.args.Args;
22+
import org.tron.core.exception.BadBlockException;
2223
import org.tron.core.exception.BadItemException;
2324
import org.tron.protos.Protocol.Transaction.Contract.ContractType;
2425
import org.tron.protos.contract.BalanceContract.TransferContract;
@@ -85,6 +86,43 @@ public void testCalcMerkleRoot() throws Exception {
8586
logger.info("Transaction[O] Merkle Root : {}", blockCapsule0.getMerkleRoot().toString());
8687
}
8788

89+
@Test
90+
public void testValidateMerkleRoot() throws Exception {
91+
// build a fresh local block so shared blockCapsule0 is not mutated
92+
String parentHash = "9938a342238077182498b464ac0292229938a342238077182498b464ac029222";
93+
BlockCapsule local = new BlockCapsule(1,
94+
Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(parentHash))),
95+
1234,
96+
ByteString.copyFrom("1234567".getBytes()));
97+
98+
// valid block: setMerkleRoot then validate — should not throw
99+
local.setMerkleRoot();
100+
local.validateMerkleRoot(); // no exception
101+
102+
// flag is set — second call must be a no-op (no recomputation)
103+
local.validateMerkleRoot(); // still no exception
104+
105+
// tamper with a transaction to break merkle
106+
TransferContract transferContract = TransferContract.newBuilder()
107+
.setAmount(999L)
108+
.setOwnerAddress(ByteString.copyFrom("0x0000000000000000000".getBytes()))
109+
.setToAddress(ByteString.copyFrom(ByteArray.fromHexString(
110+
Wallet.getAddressPreFixString() + "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
111+
.build();
112+
local.addTransaction(
113+
new TransactionCapsule(transferContract, ContractType.TransferContract));
114+
// merkle root was set before adding the tx, so it is now stale/invalid
115+
116+
BlockCapsule tampered = new BlockCapsule(local.getInstance());
117+
// tampered has no merkleValidated flag set
118+
try {
119+
tampered.validateMerkleRoot();
120+
Assert.fail("Expected BadBlockException for merkle mismatch");
121+
} catch (BadBlockException e) {
122+
Assert.assertTrue(e.getMessage().contains("merkle"));
123+
}
124+
}
125+
88126
/* @Test
89127
public void testAddTransaction() {
90128
TransactionCapsule transactionCapsule = new TransactionCapsule("123", 1L);

‎framework/src/test/java/org/tron/core/db/ManagerMockTest.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,18 @@ public void testRePush() {
372372
@Test
373373
public void testRePush1() {
374374
Manager dbManager = spy(new Manager());
375-
Protocol.Transaction transaction = Protocol.Transaction.newBuilder().build();
375+
BalanceContract.TransferContract transferContract =
376+
BalanceContract.TransferContract.newBuilder()
377+
.setOwnerAddress(ByteString.copyFromUtf8("aaa"))
378+
.setToAddress(ByteString.copyFromUtf8("bbb"))
379+
.setAmount(1)
380+
.build();
381+
Protocol.Transaction transaction = Protocol.Transaction.newBuilder()
382+
.setRawData(Protocol.Transaction.raw.newBuilder()
383+
.addContract(Protocol.Transaction.Contract.newBuilder()
384+
.setParameter(Any.pack(transferContract))
385+
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)))
386+
.build();
376387
TransactionCapsule trx = new TransactionCapsule(transaction);
377388
TransactionStore transactionStoreMock = mock(TransactionStore.class);
378389

‎framework/src/test/java/org/tron/core/db/ManagerTest.java‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -500,8 +500,8 @@ public void pushBlockInvalidMerkelRoot() {
500500
} catch (BadBlockException e) {
501501
Assert.assertTrue(e instanceof BadBlockException);
502502
Assert.assertTrue(e.getType().equals(CALC_MERKLE_ROOT_FAILED));
503-
Assert.assertEquals("The merkle hash is not validated for "
504-
+ blockCapsule2.getNum(), e.getMessage());
503+
Assert.assertTrue(e.getMessage().startsWith(
504+
"merkle root mismatch for block " + blockCapsule2.getNum() + ":"));
505505
} catch (Exception e) {
506506
Assert.assertFalse(e instanceof Exception);
507507
}
@@ -1292,6 +1292,35 @@ public void blockTrigger() {
12921292
Assert.assertEquals(TronError.ErrCode.EVENT_SUBSCRIBE_ERROR, thrown.getErrCode());
12931293
}
12941294

1295+
@Test
1296+
public void testRePushResetsVerifiedOnOwnerAddressSetHit() throws Exception {
1297+
TransferContract transferContract = TransferContract.newBuilder()
1298+
.setAmount(1L)
1299+
.setOwnerAddress(ByteString.copyFrom(
1300+
ByteArray.fromHexString(Wallet.getAddressPreFixString()
1301+
+ "548794500882809695A8A687866E76D4271A1ABC")))
1302+
.setToAddress(ByteString.copyFrom(
1303+
ByteArray.fromHexString(Wallet.getAddressPreFixString()
1304+
+ "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
1305+
.build();
1306+
TransactionCapsule tx = new TransactionCapsule(transferContract, ContractType.TransferContract);
1307+
tx.setVerified(true); // simulate mempool-cached state
1308+
1309+
String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress());
1310+
1311+
// Inject ownerAddress into ownerAddressSet via reflection
1312+
Set<String> ownerAddressSet =
1313+
(Set<String>) ReflectUtils.getFieldObject(dbManager, "ownerAddressSet");
1314+
ownerAddressSet.add(ownerAddress);
1315+
1316+
// rePush should reset isVerified to false before pushTransaction
1317+
dbManager.rePush(tx);
1318+
1319+
// After rePush, isVerified must be false
1320+
Boolean verified = (Boolean) ReflectUtils.getFieldObject(tx, "isVerified");
1321+
Assert.assertFalse(verified);
1322+
}
1323+
12951324
public void adjustBalance(AccountStore accountStore, byte[] accountAddress, long amount)
12961325
throws BalanceInsufficientException {
12971326
Commons.adjustBalance(accountStore, accountAddress, amount,

0 commit comments

Comments
 (0)