Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion common/src/main/java/org/tron/core/Constant.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public class Constant {
public static final long MAXIMUM_TIME_UNTIL_EXPIRATION = 24 * 60 * 60 * 1_000L; //one day
public static final long TRANSACTION_DEFAULT_EXPIRATION_TIME = 60 * 1_000L; //60 seconds
public static final long TRANSACTION_FEE_POOL_PERIOD = 1; //1 blocks
public static final long PER_SIGN_LENGTH = 65L;
public static final int PER_SIGN_LENGTH = 65;
public static final int MAX_PER_SIGN_LENGTH = 68;
public static final long MAX_CONTRACT_RESULT_SIZE = 2L;

// Smart contract / Energy
Expand Down
18 changes: 18 additions & 0 deletions crypto/src/main/java/org/tron/common/crypto/SignUtils.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.tron.common.crypto;

import static org.tron.core.Constant.MAX_PER_SIGN_LENGTH;
import static org.tron.core.Constant.PER_SIGN_LENGTH;

import java.security.SecureRandom;
import java.security.SignatureException;
import org.tron.common.crypto.ECKey.ECDSASignature;
Expand All @@ -8,6 +11,21 @@

public class SignUtils {

/**
* Strict signature-length check for admission entry-points (RPC broadcast,
* P2P transaction ingress, peer hello handshake). Accepts only sizes in
* [{@link org.tron.core.Constant#PER_SIGN_LENGTH PER_SIGN_LENGTH},
* {@link org.tron.core.Constant#MAX_PER_SIGN_LENGTH MAX_PER_SIGN_LENGTH}].
*
* <p>Consensus paths (e.g. {@code TransactionCapsule.checkWeight}) intentionally
* keep the looser {@code size < 65} check to remain compatible with historical
* on-chain signatures that carry trailing padding bytes; do not call this
* helper from those paths.
*/
public static boolean isValidLength(int size) {
return size >= PER_SIGN_LENGTH && size <= MAX_PER_SIGN_LENGTH;
}

public static SignInterface getGeneratedRandomSign(
SecureRandom secureRandom, boolean isECKeyCryptoEngine) {
if (isECKeyCryptoEngine) {
Expand Down
10 changes: 10 additions & 0 deletions framework/src/main/java/org/tron/core/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,16 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) {
trx.setTime(System.currentTimeMillis());
Sha256Hash txID = trx.getTransactionId();
try {
for (ByteString sig : signedTransaction.getSignatureList()) {
if (!SignUtils.isValidLength(sig.size())) {
Comment thread
Federico2014 marked this conversation as resolved.
String info = "Signature size is " + sig.size();
logger.warn("Broadcast transaction {} has failed, {}.", txID, info);
return builder.setResult(false).setCode(response_code.SIGERROR)
.setMessage(ByteString.copyFromUtf8("Validate signature error: " + info))
.build();
}
}

if (tronNetDelegate.isBlockUnsolidified()) {
logger.warn("Broadcast transaction {} has failed, block unsolidified.", txID);
return builder.setResult(false).setCode(response_code.BLOCK_UNSOLIDIFIED)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.tron.core.net.messagehandler;

import com.google.protobuf.ByteString;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand All @@ -13,6 +14,7 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.tron.common.crypto.SignUtils;
import org.tron.common.es.ExecutorServiceManager;
import org.tron.common.utils.Sha256Hash;
import org.tron.core.ChainBaseManager;
Expand Down Expand Up @@ -142,6 +144,12 @@ private void check(PeerConnection peer, TransactionsMessage msg) throws P2pExcep
throw new P2pException(TypeEnum.BAD_TRX,
"tx " + item.getHash() + " contract size should be greater than 0");
}
for (ByteString sig : trx.getSignatureList()) {
if (!SignUtils.isValidLength(sig.size())) {
throw new P2pException(TypeEnum.BAD_TRX,
"tx " + item.getHash() + " signature size is " + sig.size());
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) {
return false;
}

if (!SignUtils.isValidLength(msg.getSignature().size())) {
logger.warn("HelloMessage from {}, signature size is {}.",
channel.getInetAddress(), msg.getSignature().size());
return false;
}

boolean flag;
try {
Sha256Hash hash = Sha256Hash.of(CommonParameter
Expand Down
48 changes: 48 additions & 0 deletions framework/src/test/java/org/tron/core/WalletMockTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,54 @@ public void testCreateTransactionCapsuleWithoutValidateWithTimeout()
}


@Test
public void testBroadcastTxInvalidSigLength() throws Exception {
Wallet wallet = new Wallet();
TronNetDelegate tronNetDelegateMock = mock(TronNetDelegate.class);
Field field = wallet.getClass().getDeclaredField("tronNetDelegate");
field.setAccessible(true);
field.set(wallet, tronNetDelegateMock);

// signature shorter than 65 bytes → SIGERROR
Protocol.Transaction shortSig = Protocol.Transaction.newBuilder()
.addSignature(ByteString.copyFrom(new byte[64]))
.build();
GrpcAPI.Return ret = wallet.broadcastTransaction(shortSig);
assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode());

// signature longer than 68 bytes → SIGERROR
Protocol.Transaction longSig = Protocol.Transaction.newBuilder()
.addSignature(ByteString.copyFrom(new byte[69]))
.build();
ret = wallet.broadcastTransaction(longSig);
assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode());

// empty signature → SIGERROR
Protocol.Transaction emptySig = Protocol.Transaction.newBuilder()
.addSignature(ByteString.EMPTY)
.build();
ret = wallet.broadcastTransaction(emptySig);
assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode());

// tronNetDelegate must not be consulted because the request is rejected up front
Mockito.verify(tronNetDelegateMock, Mockito.never()).isBlockUnsolidified();

// 65-byte signature passes the length check and proceeds to downstream logic
when(tronNetDelegateMock.isBlockUnsolidified()).thenReturn(true);
Protocol.Transaction validSig = Protocol.Transaction.newBuilder()
.addSignature(ByteString.copyFrom(new byte[65]))
.build();
ret = wallet.broadcastTransaction(validSig);
assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode());

// 68-byte signature (upper bound) also passes the length check
Protocol.Transaction paddedSig = Protocol.Transaction.newBuilder()
.addSignature(ByteString.copyFrom(new byte[68]))
.build();
ret = wallet.broadcastTransaction(paddedSig);
assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode());
}

@Test
public void testBroadcastTransactionBlockUnsolidified() throws Exception {
Wallet wallet = new Wallet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,91 @@ public void testDuplicateTransactionRejected() throws Exception {
}
}

@Test
public void testInvalidSigLength() throws Exception {
TransactionsMsgHandler handler = new TransactionsMsgHandler();
handler.init();
try {
PeerConnection peer = Mockito.mock(PeerConnection.class);

Comment thread
Federico2014 marked this conversation as resolved.
BalanceContract.TransferContract transferContract = BalanceContract.TransferContract
.newBuilder()
.setAmount(10)
.setOwnerAddress(ByteString.copyFrom(ByteArray.fromHexString("121212a9cf")))
.setToAddress(ByteString.copyFrom(ByteArray.fromHexString("232323a9cf")))
.build();

// signature shorter than 65 bytes → BAD_TRX
Protocol.Transaction shortSigTrx = Protocol.Transaction.newBuilder()
.setRawData(Protocol.Transaction.raw.newBuilder()
.addContract(Protocol.Transaction.Contract.newBuilder()
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
.setParameter(Any.pack(transferContract)).build())
.build())
.addSignature(ByteString.copyFrom(new byte[64]))
.build();

List<Protocol.Transaction> shortList = new ArrayList<>();
shortList.add(shortSigTrx);
stubAdvInvRequest(peer, new TransactionsMessage(shortList));
P2pException shortEx = Assert.assertThrows(P2pException.class,
() -> handler.processMessage(peer, new TransactionsMessage(shortList)));
Assert.assertEquals(TypeEnum.BAD_TRX, shortEx.getType());

// signature longer than 68 bytes → BAD_TRX
Protocol.Transaction longSigTrx = Protocol.Transaction.newBuilder()
.setRawData(Protocol.Transaction.raw.newBuilder()
.setRefBlockNum(1)
.addContract(Protocol.Transaction.Contract.newBuilder()
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
.setParameter(Any.pack(transferContract)).build())
.build())
.addSignature(ByteString.copyFrom(new byte[69]))
.build();

List<Protocol.Transaction> longList = new ArrayList<>();
longList.add(longSigTrx);
stubAdvInvRequest(peer, new TransactionsMessage(longList));
P2pException longEx = Assert.assertThrows(P2pException.class,
() -> handler.processMessage(peer, new TransactionsMessage(longList)));
Assert.assertEquals(TypeEnum.BAD_TRX, longEx.getType());

// exactly 65 bytes → passes the length check (no P2pException from check)
Protocol.Transaction validSigTrx = Protocol.Transaction.newBuilder()
.setRawData(Protocol.Transaction.raw.newBuilder()
.setRefBlockNum(2)
.addContract(Protocol.Transaction.Contract.newBuilder()
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
.setParameter(Any.pack(transferContract)).build())
.build())
.addSignature(ByteString.copyFrom(new byte[65]))
.build();

List<Protocol.Transaction> validList = new ArrayList<>();
validList.add(validSigTrx);
stubAdvInvRequest(peer, new TransactionsMessage(validList));
handler.processMessage(peer, new TransactionsMessage(validList));

// 68 bytes (upper bound) also passes the length check
Protocol.Transaction paddedSigTrx = Protocol.Transaction.newBuilder()
.setRawData(Protocol.Transaction.raw.newBuilder()
.setRefBlockNum(3)
.addContract(Protocol.Transaction.Contract.newBuilder()
.setType(Protocol.Transaction.Contract.ContractType.TransferContract)
.setParameter(Any.pack(transferContract)).build())
.build())
.addSignature(ByteString.copyFrom(new byte[68]))
.build();

List<Protocol.Transaction> paddedList = new ArrayList<>();
paddedList.add(paddedSigTrx);
stubAdvInvRequest(peer, new TransactionsMessage(paddedList));
handler.processMessage(peer, new TransactionsMessage(paddedList));
} finally {
handler.close();
}
}

@Test
public void testIsBusyWithCachedTransactions() throws Exception {
TransactionsMsgHandler handler = new TransactionsMsgHandler();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,30 @@ private void testCheckHelloMessage() {

boolean res = service.checkHelloMessage(helloMessage, c1);
Assert.assertTrue(res);

HelloMessage shortSigMsg = new HelloMessage(node, System.currentTimeMillis(),
ChainBaseManager.getChainBaseManager());
shortSigMsg.setHelloMessage(shortSigMsg.getHelloMessage().toBuilder()
.setAddress(address)
.setSignature(ByteString.copyFrom(new byte[64]))
.build());
Assert.assertFalse(service.checkHelloMessage(shortSigMsg, c1));

HelloMessage longSigMsg = new HelloMessage(node, System.currentTimeMillis(),
ChainBaseManager.getChainBaseManager());
longSigMsg.setHelloMessage(longSigMsg.getHelloMessage().toBuilder()
.setAddress(address)
.setSignature(ByteString.copyFrom(new byte[69]))
.build());
Assert.assertFalse(service.checkHelloMessage(longSigMsg, c1));

HelloMessage emptySigMsg = new HelloMessage(node, System.currentTimeMillis(),
ChainBaseManager.getChainBaseManager());
emptySigMsg.setHelloMessage(emptySigMsg.getHelloMessage().toBuilder()
.setAddress(address)
.setSignature(ByteString.EMPTY)
.build());
Assert.assertFalse(service.checkHelloMessage(emptySigMsg, c1));
} catch (Exception e) {
logger.info("", e);
assert false;
Expand Down
Loading