Skip to content

fix(net): enforce 65-byte signature length#6782

Open
Federico2014 wants to merge 1 commit into
tronprotocol:release_v4.8.2from
Federico2014:fix/limit_sig_length_65
Open

fix(net): enforce 65-byte signature length#6782
Federico2014 wants to merge 1 commit into
tronprotocol:release_v4.8.2from
Federico2014:fix/limit_sig_length_65

Conversation

@Federico2014
Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds a strict 65-byte length check on signatures received at broadcast / P2P admission entrypoints, before any signature recovery work is done:

  • Wallet.broadcastTransaction rejects the transaction with SIGERROR when any signature in the list is not 65 bytes.
  • TransactionsMsgHandler.check throws P2pException(BAD_TRX) when any signature in a TransactionsMessage is not 65 bytes, causing the peer to be disconnected with BAD_TX.
  • RelayService.checkHelloMessage returns false when the HelloMessage signature from a fast-forward peer is not 65 bytes.

The constant Constant.PER_SIGN_LENGTH (= 65) is reused everywhere.

Why are these changes required?

A TRON transaction / hello signature is encoded as 65 bytes (r 32 + s 32 + v 1). The affected entrypoints currently pass malformed admission payloads into SignUtils.signatureToAddress / signature recovery first, which wastes CPU before the request is rejected or fails later.

This PR adds a cheap length check at the network and broadcast boundaries so invalid admission payloads fail before crypto recovery is attempted. This is intentionally scoped to admission handling only.

Consensus compatibility

This PR does not change transaction validation for blocks or any shared consensus validation path.

In particular, TransactionCapsule.checkWeight currently rejects signatures shorter than 65 bytes, while longer signatures are parsed by the existing Rsv.fromSignature behavior using the first 65 bytes. Tightening that shared validation path would be a consensus-impacting behavior change and is intentionally not part of this PR.

The observable behavior change is limited to local broadcast / P2P admission:

  • Clients broadcasting transactions with non-65-byte signatures now receive SIGERROR before downstream checks.
  • Peers sending transaction messages with non-65-byte signatures are rejected at message admission with BAD_TX.
  • Fast-forward hello messages with non-65-byte signatures are rejected before signature recovery.

This PR has been tested by:

  • Unit Tests
    • WalletMockTest#testBroadcastTxInvalidSigLength: 64 / 66 / empty signatures return SIGERROR; 65-byte signature falls through.
    • TransactionsMsgHandlerTest#testInvalidSigLength: 64 / 66 / empty signatures raise P2pException(BAD_TRX) via assertThrows; 65-byte signature passes the new length check.
    • RelayServiceTest#testCheckHelloMessage: extended to assert that 64 / 66 / empty HelloMessage signatures return false; the existing 65-byte case still returns true.
  • Manual Testing
    • ./gradlew :framework:checkstyleMain :framework:checkstyleTest
    • ./gradlew :framework:test --tests "org.tron.core.WalletMockTest.testBroadcastTxInvalidSigLength" --tests "org.tron.core.net.messagehandler.TransactionsMsgHandlerTest.testInvalidSigLength" --tests "org.tron.core.net.services.RelayServiceTest.testCheckHelloMessage"

Follow up

None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants