Skip to content

Commit ace64b0

Browse files
Federico2014Federico2014
committed
fix(net): enforce 65-byte signature length
1 parent 5f7eeca commit ace64b0

6 files changed

Lines changed: 162 additions & 0 deletions

File tree

‎framework/src/main/java/org/tron/core/Wallet.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static org.tron.common.utils.Commons.getAssetIssueStoreFinal;
2525
import static org.tron.common.utils.Commons.getExchangeStoreFinal;
2626
import static org.tron.common.utils.WalletUtil.isConstant;
27+
import static org.tron.core.Constant.PER_SIGN_LENGTH;
2728
import static org.tron.core.capsule.utils.TransactionUtil.buildInternalTransaction;
2829
import static org.tron.core.config.Parameter.ChainConstant.BLOCK_PRODUCED_INTERVAL;
2930
import static org.tron.core.config.Parameter.ChainConstant.TRX_PRECISION;
@@ -505,6 +506,16 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) {
505506
trx.setTime(System.currentTimeMillis());
506507
Sha256Hash txID = trx.getTransactionId();
507508
try {
509+
for (ByteString sig : signedTransaction.getSignatureList()) {
510+
if (sig.size() != PER_SIGN_LENGTH) {
511+
String info = "Signature size is " + sig.size();
512+
logger.warn("Broadcast transaction {} has failed, {}.", txID, info);
513+
return builder.setResult(false).setCode(response_code.SIGERROR)
514+
.setMessage(ByteString.copyFromUtf8("Validate signature error: " + info))
515+
.build();
516+
}
517+
}
518+
508519
if (tronNetDelegate.isBlockUnsolidified()) {
509520
logger.warn("Broadcast transaction {} has failed, block unsolidified.", txID);
510521
return builder.setResult(false).setCode(response_code.BLOCK_UNSOLIDIFIED)

‎framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java‎

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

3+
import static org.tron.core.Constant.PER_SIGN_LENGTH;
4+
5+
import com.google.protobuf.ByteString;
36
import java.util.HashSet;
47
import java.util.List;
58
import java.util.Set;
@@ -142,6 +145,12 @@ private void check(PeerConnection peer, TransactionsMessage msg) throws P2pExcep
142145
throw new P2pException(TypeEnum.BAD_TRX,
143146
"tx " + item.getHash() + " contract size should be greater than 0");
144147
}
148+
for (ByteString sig : trx.getSignatureList()) {
149+
if (sig.size() != PER_SIGN_LENGTH) {
150+
throw new P2pException(TypeEnum.BAD_TRX,
151+
"tx " + item.getHash() + " signature size is " + sig.size());
152+
}
153+
}
145154
}
146155
}
147156

‎framework/src/main/java/org/tron/core/net/service/relay/RelayService.java‎

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

3+
import static org.tron.core.Constant.PER_SIGN_LENGTH;
4+
35
import com.google.protobuf.ByteString;
46
import java.net.InetSocketAddress;
57
import java.util.Arrays;
@@ -150,6 +152,12 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) {
150152
return false;
151153
}
152154

155+
if (msg.getSignature().size() != PER_SIGN_LENGTH) {
156+
logger.warn("HelloMessage from {}, signature size is {}.",
157+
channel.getInetAddress(), msg.getSignature().size());
158+
return false;
159+
}
160+
153161
boolean flag;
154162
try {
155163
Sha256Hash hash = Sha256Hash.of(CommonParameter

‎framework/src/test/java/org/tron/core/WalletMockTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,47 @@ public void testCreateTransactionCapsuleWithoutValidateWithTimeout()
164164
}
165165

166166

167+
@Test
168+
public void testBroadcastTxInvalidSigLength() throws Exception {
169+
Wallet wallet = new Wallet();
170+
TronNetDelegate tronNetDelegateMock = mock(TronNetDelegate.class);
171+
Field field = wallet.getClass().getDeclaredField("tronNetDelegate");
172+
field.setAccessible(true);
173+
field.set(wallet, tronNetDelegateMock);
174+
175+
// signature shorter than 65 bytes → SIGERROR
176+
Protocol.Transaction shortSig = Protocol.Transaction.newBuilder()
177+
.addSignature(ByteString.copyFrom(new byte[64]))
178+
.build();
179+
GrpcAPI.Return ret = wallet.broadcastTransaction(shortSig);
180+
assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode());
181+
182+
// signature longer than 65 bytes → SIGERROR
183+
Protocol.Transaction longSig = Protocol.Transaction.newBuilder()
184+
.addSignature(ByteString.copyFrom(new byte[66]))
185+
.build();
186+
ret = wallet.broadcastTransaction(longSig);
187+
assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode());
188+
189+
// empty signature → SIGERROR
190+
Protocol.Transaction emptySig = Protocol.Transaction.newBuilder()
191+
.addSignature(ByteString.EMPTY)
192+
.build();
193+
ret = wallet.broadcastTransaction(emptySig);
194+
assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode());
195+
196+
// tronNetDelegate must not be consulted because the request is rejected up front
197+
Mockito.verify(tronNetDelegateMock, Mockito.never()).isBlockUnsolidified();
198+
199+
// 65-byte signature passes the length check and proceeds to downstream logic
200+
when(tronNetDelegateMock.isBlockUnsolidified()).thenReturn(true);
201+
Protocol.Transaction validSig = Protocol.Transaction.newBuilder()
202+
.addSignature(ByteString.copyFrom(new byte[65]))
203+
.build();
204+
ret = wallet.broadcastTransaction(validSig);
205+
assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode());
206+
}
207+
167208
@Test
168209
public void testBroadcastTransactionBlockUnsolidified() throws Exception {
169210
Wallet wallet = new Wallet();

‎framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,75 @@ public void testDuplicateTransactionRejected() throws Exception {
337337
}
338338
}
339339

340+
@Test
341+
public void testInvalidSigLength() throws Exception {
342+
TransactionsMsgHandler handler = new TransactionsMsgHandler();
343+
handler.init();
344+
try {
345+
PeerConnection peer = Mockito.mock(PeerConnection.class);
346+
347+
BalanceContract.TransferContract transferContract = BalanceContract.TransferContract
348+
.newBuilder()
349+
.setAmount(10)
350+
.setOwnerAddress(ByteString.copyFrom(ByteArray.fromHexString("121212a9cf")))
351+
.setToAddress(ByteString.copyFrom(ByteArray.fromHexString("232323a9cf")))
352+
.build();
353+
354+
// signature shorter than 65 bytes → BAD_TRX
355+
Protocol.Transaction shortSigTrx = Protocol.Transaction.newBuilder()
356+
.setRawData(Protocol.Transaction.raw.newBuilder()
357+
.addContract(Protocol.Transaction.Contract.newBuilder()
358+
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
359+
.setParameter(Any.pack(transferContract)).build())
360+
.build())
361+
.addSignature(ByteString.copyFrom(new byte[64]))
362+
.build();
363+
364+
List<Protocol.Transaction> shortList = new ArrayList<>();
365+
shortList.add(shortSigTrx);
366+
stubAdvInvRequest(peer, new TransactionsMessage(shortList));
367+
P2pException shortEx = Assert.assertThrows(P2pException.class,
368+
() -> handler.processMessage(peer, new TransactionsMessage(shortList)));
369+
Assert.assertEquals(TypeEnum.BAD_TRX, shortEx.getType());
370+
371+
// signature longer than 65 bytes → BAD_TRX
372+
Protocol.Transaction longSigTrx = Protocol.Transaction.newBuilder()
373+
.setRawData(Protocol.Transaction.raw.newBuilder()
374+
.setRefBlockNum(1)
375+
.addContract(Protocol.Transaction.Contract.newBuilder()
376+
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
377+
.setParameter(Any.pack(transferContract)).build())
378+
.build())
379+
.addSignature(ByteString.copyFrom(new byte[66]))
380+
.build();
381+
382+
List<Protocol.Transaction> longList = new ArrayList<>();
383+
longList.add(longSigTrx);
384+
stubAdvInvRequest(peer, new TransactionsMessage(longList));
385+
P2pException longEx = Assert.assertThrows(P2pException.class,
386+
() -> handler.processMessage(peer, new TransactionsMessage(longList)));
387+
Assert.assertEquals(TypeEnum.BAD_TRX, longEx.getType());
388+
389+
// exactly 65 bytes → passes the length check (no P2pException from check)
390+
Protocol.Transaction validSigTrx = Protocol.Transaction.newBuilder()
391+
.setRawData(Protocol.Transaction.raw.newBuilder()
392+
.setRefBlockNum(2)
393+
.addContract(Protocol.Transaction.Contract.newBuilder()
394+
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
395+
.setParameter(Any.pack(transferContract)).build())
396+
.build())
397+
.addSignature(ByteString.copyFrom(new byte[65]))
398+
.build();
399+
400+
List<Protocol.Transaction> validList = new ArrayList<>();
401+
validList.add(validSigTrx);
402+
stubAdvInvRequest(peer, new TransactionsMessage(validList));
403+
handler.processMessage(peer, new TransactionsMessage(validList));
404+
} finally {
405+
handler.close();
406+
}
407+
}
408+
340409
@Test
341410
public void testIsBusyWithCachedTransactions() throws Exception {
342411
TransactionsMsgHandler handler = new TransactionsMsgHandler();

‎framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,30 @@ private void testCheckHelloMessage() {
220220

221221
boolean res = service.checkHelloMessage(helloMessage, c1);
222222
Assert.assertTrue(res);
223+
224+
HelloMessage shortSigMsg = new HelloMessage(node, System.currentTimeMillis(),
225+
ChainBaseManager.getChainBaseManager());
226+
shortSigMsg.setHelloMessage(shortSigMsg.getHelloMessage().toBuilder()
227+
.setAddress(address)
228+
.setSignature(ByteString.copyFrom(new byte[64]))
229+
.build());
230+
Assert.assertFalse(service.checkHelloMessage(shortSigMsg, c1));
231+
232+
HelloMessage longSigMsg = new HelloMessage(node, System.currentTimeMillis(),
233+
ChainBaseManager.getChainBaseManager());
234+
longSigMsg.setHelloMessage(longSigMsg.getHelloMessage().toBuilder()
235+
.setAddress(address)
236+
.setSignature(ByteString.copyFrom(new byte[66]))
237+
.build());
238+
Assert.assertFalse(service.checkHelloMessage(longSigMsg, c1));
239+
240+
HelloMessage emptySigMsg = new HelloMessage(node, System.currentTimeMillis(),
241+
ChainBaseManager.getChainBaseManager());
242+
emptySigMsg.setHelloMessage(emptySigMsg.getHelloMessage().toBuilder()
243+
.setAddress(address)
244+
.setSignature(ByteString.EMPTY)
245+
.build());
246+
Assert.assertFalse(service.checkHelloMessage(emptySigMsg, c1));
223247
} catch (Exception e) {
224248
logger.info("", e);
225249
assert false;

0 commit comments

Comments
 (0)