Skip to content

Commit 2d44164

Browse files
wbwb
authored andcommitted
fix(security): re-verify block signature during fork replay
- Manager.switchFork now re-validates each replayed block's witness signature before applying. The witness account's permission can change between forks (via permission-update transactions), so a signature that was valid on the original chain may no longer be valid on the replay path. - Use `if (!validateSignature(...)) throw new ValidateSignatureException` rather than discarding the boolean return: validateSignature only throws on malformed signature bytes; an attacker-supplied valid-format signature with a wrong-signer address returns false. Discarding the return would let that attack through. - The existing switchFork catch list already includes ValidateSignatureException, so the new throw is wired into the existing switchback path with no additional handling. - Add three BlockCapsule.validateSignature contract tests pinning the two failure modes the fix relies on: signer-mismatch returns false, signer-match returns true, and a 65-byte malformed signature throws ValidateSignatureException.
1 parent 260585c commit 2d44164

2 files changed

Lines changed: 102 additions & 0 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,6 +1140,11 @@ private void switchFork(BlockCapsule newHead)
11401140
Exception exception = null;
11411141
// todo process the exception carefully later
11421142
try (ISession tmpSession = revokingStore.buildSession()) {
1143+
if (!item.getBlk().validateSignature(
1144+
getDynamicPropertiesStore(), getAccountStore())) {
1145+
throw new ValidateSignatureException(
1146+
"switch fork: block " + item.getBlk().getNum() + " signature invalid");
1147+
}
11431148
applyBlock(item.getBlk().setSwitch(true));
11441149
tmpSession.commit();
11451150
} catch (AccountResourceInsufficientException

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package org.tron.core.capsule;
22

3+
import static org.mockito.Mockito.mock;
4+
import static org.mockito.Mockito.when;
5+
36
import com.google.protobuf.ByteString;
47
import java.io.IOException;
58
import java.util.ArrayList;
@@ -21,6 +24,11 @@
2124
import org.tron.core.config.args.Args;
2225
import org.tron.core.exception.BadBlockException;
2326
import org.tron.core.exception.BadItemException;
27+
import org.tron.core.exception.ValidateSignatureException;
28+
import org.tron.core.store.AccountStore;
29+
import org.tron.core.store.DynamicPropertiesStore;
30+
import org.tron.protos.Protocol.Block;
31+
import org.tron.protos.Protocol.BlockHeader;
2432
import org.tron.protos.Protocol.Transaction.Contract.ContractType;
2533
import org.tron.protos.contract.BalanceContract.TransferContract;
2634

@@ -180,6 +188,95 @@ public void testGetTimeStamp() {
180188
Assert.assertEquals(1234L, blockCapsule0.getTimeStamp());
181189
}
182190

191+
/**
192+
* Pin the contract that switchFork's signature recheck relies on:
193+
* when the recovered signer address does not match the witness address,
194+
* validateSignature returns false (no exception). switchFork uses the
195+
* boolean return to decide whether to throw, so this contract is what
196+
* makes the fix work for "wrong signer" attacks.
197+
*/
198+
@Test
199+
public void testValidateSignatureReturnsFalseWhenSignerMismatch() throws Exception {
200+
String signerKey = PublicMethod.getRandomPrivateKey();
201+
String witnessKey = PublicMethod.getRandomPrivateKey();
202+
byte[] witnessAddress = PublicMethod.getAddressByteByPrivateKey(witnessKey);
203+
204+
BlockCapsule block = new BlockCapsule(2,
205+
Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(
206+
"9938a342238077182498b464ac0292229938a342238077182498b464ac029222"))),
207+
4321,
208+
ByteString.copyFrom(witnessAddress));
209+
block.sign(ByteArray.fromHexString(signerKey));
210+
211+
DynamicPropertiesStore dps = mock(DynamicPropertiesStore.class);
212+
when(dps.getAllowMultiSign()).thenReturn(0L);
213+
AccountStore accountStore = mock(AccountStore.class);
214+
215+
Assert.assertFalse(block.validateSignature(dps, accountStore));
216+
}
217+
218+
/**
219+
* Same key path under the happy case: when signer == witness, validateSignature
220+
* returns true. Guards against any future refactor that accidentally inverts
221+
* the comparison or strips the witness check.
222+
*/
223+
@Test
224+
public void testValidateSignatureReturnsTrueWhenSignerMatches() throws Exception {
225+
String key = PublicMethod.getRandomPrivateKey();
226+
byte[] witnessAddress = PublicMethod.getAddressByteByPrivateKey(key);
227+
228+
BlockCapsule block = new BlockCapsule(3,
229+
Sha256Hash.wrap(ByteString.copyFrom(ByteArray.fromHexString(
230+
"9938a342238077182498b464ac0292229938a342238077182498b464ac029222"))),
231+
5678,
232+
ByteString.copyFrom(witnessAddress));
233+
block.sign(ByteArray.fromHexString(key));
234+
235+
DynamicPropertiesStore dps = mock(DynamicPropertiesStore.class);
236+
when(dps.getAllowMultiSign()).thenReturn(0L);
237+
AccountStore accountStore = mock(AccountStore.class);
238+
239+
Assert.assertTrue(block.validateSignature(dps, accountStore));
240+
}
241+
242+
/**
243+
* The other failure mode switchFork must handle: signature bytes are
244+
* malformed (cannot recover a public key). validateSignature wraps the
245+
* underlying SignatureException as ValidateSignatureException, which the
246+
* existing catch block in switchFork already handles.
247+
*/
248+
@Test(expected = ValidateSignatureException.class)
249+
public void testValidateSignatureThrowsForMalformedSignature() throws Exception {
250+
byte[] witnessAddress = PublicMethod.getAddressByteByPrivateKey(
251+
PublicMethod.getRandomPrivateKey());
252+
253+
// 65-byte signature with valid length but garbage content — passes Rsv parsing
254+
// but fails ECDSA recovery, surfacing SignatureException → ValidateSignatureException.
255+
byte[] garbageSigBytes = new byte[65];
256+
Arrays.fill(garbageSigBytes, (byte) 0xAB);
257+
ByteString garbageSig = ByteString.copyFrom(garbageSigBytes);
258+
259+
BlockHeader.raw rawData = BlockHeader.raw.newBuilder()
260+
.setNumber(4)
261+
.setTimestamp(1111)
262+
.setParentHash(ByteString.copyFrom(ByteArray.fromHexString(
263+
"9938a342238077182498b464ac0292229938a342238077182498b464ac029222")))
264+
.setWitnessAddress(ByteString.copyFrom(witnessAddress))
265+
.build();
266+
BlockHeader header = BlockHeader.newBuilder()
267+
.setRawData(rawData)
268+
.setWitnessSignature(garbageSig)
269+
.build();
270+
Block proto = Block.newBuilder().setBlockHeader(header).build();
271+
BlockCapsule block = new BlockCapsule(proto);
272+
273+
DynamicPropertiesStore dps = mock(DynamicPropertiesStore.class);
274+
when(dps.getAllowMultiSign()).thenReturn(0L);
275+
AccountStore accountStore = mock(AccountStore.class);
276+
277+
block.validateSignature(dps, accountStore);
278+
}
279+
183280
@Test
184281
public void testConcurrentToString() throws InterruptedException {
185282
List<Thread> threadList = new ArrayList<>();

0 commit comments

Comments
 (0)