Skip to content

Commit c9ed97f

Browse files
wbwb
authored andcommitted
fix(security): validate merkle root before broadcast and reset isVerified on permission revocation
1 parent 9529fb8 commit c9ed97f

7 files changed

Lines changed: 152 additions & 9 deletions

File tree

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
package org.tron.core.capsule;
1717

18+
import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED;
19+
1820
import com.google.common.primitives.Longs;
1921
import com.google.protobuf.ByteString;
2022
import com.google.protobuf.CodedInputStream;
@@ -37,6 +39,7 @@
3739
import org.tron.common.utils.Time;
3840
import org.tron.core.capsule.utils.MerkleTree;
3941
import org.tron.core.config.Parameter.ChainConstant;
42+
import org.tron.core.exception.BadBlockException;
4043
import org.tron.core.exception.BadItemException;
4144
import org.tron.core.exception.ValidateSignatureException;
4245
import org.tron.core.store.AccountStore;
@@ -49,6 +52,7 @@
4952
public class BlockCapsule implements ProtoCapsule<Block> {
5053

5154
public boolean generatedByMyself = false;
55+
private volatile boolean merkleValidated = false;
5256
@Getter
5357
@Setter
5458
private TransactionRetCapsule result;
@@ -225,6 +229,19 @@ public Sha256Hash calcMerkleRoot() {
225229
return MerkleTree.getInstance().createTree(ids).getRoot().getHash();
226230
}
227231

232+
public void validateMerkleRoot() throws BadBlockException {
233+
if (merkleValidated) {
234+
return;
235+
}
236+
Sha256Hash actual = calcMerkleRoot();
237+
if (!actual.equals(getMerkleRoot())) {
238+
throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
239+
String.format("merkle root mismatch for block %d: expected %s, actual %s",
240+
getNum(), getMerkleRoot(), actual));
241+
}
242+
merkleValidated = true;
243+
}
244+
228245
public void setMerkleRoot() {
229246
BlockHeader.raw blockHeaderRaw =
230247
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
@@ -1307,12 +1307,7 @@ public void pushBlock(final BlockCapsule block)
13071307
try (PendingManager pm = new PendingManager(this)) {
13081308

13091309
if (!block.generatedByMyself) {
1310-
if (!block.calcMerkleRoot().equals(block.getMerkleRoot())) {
1311-
logger.warn("Num: {}, the merkle root doesn't match, expect is {} , actual is {}.",
1312-
block.getNum(), block.getMerkleRoot(), block.calcMerkleRoot());
1313-
throw new BadBlockException(CALC_MERKLE_ROOT_FAILED,
1314-
String.format("The merkle hash is not validated for %d", block.getNum()));
1315-
}
1310+
block.validateMerkleRoot();
13161311
consensus.receiveBlock(block);
13171312
}
13181313

@@ -2126,6 +2121,13 @@ public void rePush(TransactionCapsule tx) {
21262121
return;
21272122
}
21282123

2124+
String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress());
2125+
synchronized (this) {
2126+
if (ownerAddressSet.contains(ownerAddress)) {
2127+
tx.setVerified(false);
2128+
}
2129+
}
2130+
21292131
try {
21302132
this.pushTransaction(tx);
21312133
} 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
@@ -380,7 +380,18 @@ public void testRePush() {
380380
@Test
381381
public void testRePush1() {
382382
Manager dbManager = spy(new Manager());
383-
Protocol.Transaction transaction = Protocol.Transaction.newBuilder().build();
383+
BalanceContract.TransferContract transferContract =
384+
BalanceContract.TransferContract.newBuilder()
385+
.setOwnerAddress(ByteString.copyFromUtf8("aaa"))
386+
.setToAddress(ByteString.copyFromUtf8("bbb"))
387+
.setAmount(1)
388+
.build();
389+
Protocol.Transaction transaction = Protocol.Transaction.newBuilder()
390+
.setRawData(Protocol.Transaction.raw.newBuilder()
391+
.addContract(Protocol.Transaction.Contract.newBuilder()
392+
.setParameter(Any.pack(transferContract))
393+
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)))
394+
.build();
384395
TransactionCapsule trx = new TransactionCapsule(transaction);
385396
TransactionStore transactionStoreMock = mock(TransactionStore.class);
386397

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,8 @@ public void pushBlockInvalidMerkelRoot() {
508508
} catch (BadBlockException e) {
509509
Assert.assertTrue(e instanceof BadBlockException);
510510
Assert.assertTrue(e.getType().equals(CALC_MERKLE_ROOT_FAILED));
511-
Assert.assertEquals("The merkle hash is not validated for "
512-
+ blockCapsule2.getNum(), e.getMessage());
511+
Assert.assertTrue(e.getMessage().startsWith(
512+
"merkle root mismatch for block " + blockCapsule2.getNum() + ":"));
513513
} catch (Exception e) {
514514
Assert.assertFalse(e instanceof Exception);
515515
}
@@ -1419,6 +1419,35 @@ public void testClearSolidityContractTriggerCache() throws Exception {
14191419
}
14201420
}
14211421

1422+
@Test
1423+
public void testRePushResetsVerifiedOnOwnerAddressSetHit() throws Exception {
1424+
TransferContract transferContract = TransferContract.newBuilder()
1425+
.setAmount(1L)
1426+
.setOwnerAddress(ByteString.copyFrom(
1427+
ByteArray.fromHexString(Wallet.getAddressPreFixString()
1428+
+ "548794500882809695A8A687866E76D4271A1ABC")))
1429+
.setToAddress(ByteString.copyFrom(
1430+
ByteArray.fromHexString(Wallet.getAddressPreFixString()
1431+
+ "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
1432+
.build();
1433+
TransactionCapsule tx = new TransactionCapsule(transferContract, ContractType.TransferContract);
1434+
tx.setVerified(true); // simulate mempool-cached state
1435+
1436+
String ownerAddress = ByteArray.toHexString(tx.getOwnerAddress());
1437+
1438+
// Inject ownerAddress into ownerAddressSet via reflection
1439+
Set<String> ownerAddressSet =
1440+
(Set<String>) ReflectUtils.getFieldObject(dbManager, "ownerAddressSet");
1441+
ownerAddressSet.add(ownerAddress);
1442+
1443+
// rePush should reset isVerified to false before pushTransaction
1444+
dbManager.rePush(tx);
1445+
1446+
// After rePush, isVerified must be false
1447+
Boolean verified = (Boolean) ReflectUtils.getFieldObject(tx, "isVerified");
1448+
Assert.assertFalse(verified);
1449+
}
1450+
14221451
public void adjustBalance(AccountStore accountStore, byte[] accountAddress, long amount)
14231452
throws BalanceInsufficientException {
14241453
Commons.adjustBalance(accountStore, accountAddress, amount,

framework/src/test/java/org/tron/core/net/TronNetDelegateTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,24 @@
22

33
import static org.mockito.Mockito.mock;
44

5+
import com.google.protobuf.ByteString;
56
import java.lang.reflect.Field;
67
import org.junit.Assert;
78
import org.junit.Test;
89
import org.mockito.Mockito;
910
import org.tron.common.TestConstants;
1011
import org.tron.common.parameter.CommonParameter;
12+
import org.tron.common.utils.ByteArray;
1113
import org.tron.common.utils.Sha256Hash;
1214
import org.tron.core.ChainBaseManager;
15+
import org.tron.core.Wallet;
1316
import org.tron.core.capsule.BlockCapsule;
17+
import org.tron.core.capsule.TransactionCapsule;
1418
import org.tron.core.config.args.Args;
19+
import org.tron.core.exception.P2pException;
20+
import org.tron.core.exception.P2pException.TypeEnum;
21+
import org.tron.protos.Protocol.Transaction.Contract.ContractType;
22+
import org.tron.protos.contract.BalanceContract.TransferContract;
1523

1624
public class TronNetDelegateTest {
1725

@@ -49,4 +57,37 @@ public void test() throws Exception {
4957

5058
Assert.assertTrue(!tronNetDelegate.isBlockUnsolidified());
5159
}
60+
61+
@Test
62+
public void testValidBlockMerkleRoot() throws Exception {
63+
Args.setParam(new String[] {}, TestConstants.TEST_CONF);
64+
65+
String parentHash = "9938a342238077182498b464ac0292229938a342238077182498b464ac029222";
66+
BlockCapsule block = new BlockCapsule(1,
67+
Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(parentHash))),
68+
System.currentTimeMillis(),
69+
ByteString.copyFrom("witness".getBytes()));
70+
block.setMerkleRoot();
71+
72+
// Add a transaction after setMerkleRoot, making the stored merkle root stale.
73+
TransferContract transferContract = TransferContract.newBuilder()
74+
.setAmount(1L)
75+
.setOwnerAddress(ByteString.copyFrom("0x0000000000000000000".getBytes()))
76+
.setToAddress(ByteString.copyFrom(ByteArray.fromHexString(
77+
Wallet.getAddressPreFixString() + "A389132D6639FBDA4FBC8B659264E6B7C90DB086")))
78+
.build();
79+
block.addTransaction(
80+
new TransactionCapsule(transferContract, ContractType.TransferContract));
81+
82+
// Wrap in a fresh BlockCapsule so the merkleValidated flag is reset.
83+
BlockCapsule tampered = new BlockCapsule(block.getInstance());
84+
85+
TronNetDelegate tronNetDelegate = new TronNetDelegate();
86+
try {
87+
tronNetDelegate.validBlock(tampered);
88+
Assert.fail("Expected P2pException for tampered merkle root");
89+
} catch (P2pException e) {
90+
Assert.assertEquals(TypeEnum.BLOCK_MERKLE_ERROR, e.getType());
91+
}
92+
}
5293
}

0 commit comments

Comments
 (0)