Skip to content

fix(crypto): reject infinity in point address derivation - #66

Open
Federico2014 wants to merge 3 commits into
release_v4.8.3from
fix/hash-address-point-validation
Open

Federico2014 wants to merge 3 commits into
release_v4.8.3from
fix/hash-address-point-validation

Conversation

@Federico2014

@Federico2014 Federico2014 commented Nov 2, 2026 •

Copy link
Copy Markdown
Owner

What does this PR do?

Reject the point at infinity in Hash.computeAddress(ECPoint) with IllegalArgumentException. Document the uncompressed public-key input format of the byte[] overload.

Why are these changes required?

The infinity encoding is {0x00}. Previously, removing its first byte caused address derivation to hash empty input and return a well-formed address for an invalid public key.

This PR has been tested by:

  • 39 tests passed across HashTest, ECKeyTest, and BouncyCastleTest, including 3 new regression tests for infinity rejection, known public-key addresses, and legacy byte-array behavior.
  • Passed :framework:checkstyleMain :framework:checkstyleTest :plugins:checkstyleMain, check_math_usage.sh, and git diff --check.
  • Full test suite and manual node testing were not run.

Follow up

None.

Extra details

Transaction/block signature verification and TVM address recovery use the unchanged byte[] overload. Existing ECKey construction already rejects infinity, so normal getAddress() behavior is preserved. Direct callers passing infinity to the ECPoint overload now receive an exception; null-input behavior is unchanged.


Summary by cubic

Rejects the point at infinity in Hash.computeAddress(ECPoint) so invalid public keys no longer produce a well-formed address, and documents the input contract of the byte[] overload.

  • The infinity encoding {0x00} previously lost its first byte and hashed empty input, returning a valid-looking address for an invalid public key.
  • The ECPoint overload now throws IllegalArgumentException for infinity; the byte[] overload keeps its legacy behavior, which consensus paths such as signature recovery rely on.
  • Adds regression tests covering infinity rejection, known public-key addresses, and legacy byte-array behavior.

Written for commit 8e726af. Summary will update on new commits.

View guided diff Turn on auto-fix

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d35d5430-5f18-4172-b431-45b74e4cd832

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crypto/src/main/java/org/tron/common/crypto/Hash.java">

<violation number="1" location="crypto/src/main/java/org/tron/common/crypto/Hash.java:194">
P2: This Javadoc incorrectly presents the byte-array overload as accepting only 65-byte uncompressed keys, while legacy signature-recovery paths still intentionally pass encodings such as `{0x00}`. Document the unchanged no-validation behavior and the legacy compatibility requirement so callers do not reject valid consensus inputs or add incompatible validation later.</violation>
</file>

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.

1 participant