Skip to content

fix(abi,utils): harden decoder input bounds and crypto argument valid - #219

Merged
kuny0707 merged 2 commits into
tronprotocol:release_1.0.0from
0xbigapple:fix/audit_risk_v2
Jul 22, 2026
Merged

fix(abi,utils): harden decoder input bounds and crypto argument valid#219
kuny0707 merged 2 commits into
tronprotocol:release_1.0.0from
0xbigapple:fix/audit_risk_v2

Conversation

@0xbigapple

Copy link
Copy Markdown
Collaborator

What does this PR do?

  1. Hardens ABI decoder input validation (abi module): adds bounds checks, applied before use, for the lengths and offsets read from untrusted return data / event logs. Malformed input now fails fast with a uniform IllegalArgumentException instead of integer overflow, StringIndexOutOfBoundsException, NegativeArraySizeException, or an oversized memory pre-allocation (DoS).
  2. Hardens the SECP256K1 crypto utilities (utils module): replaces the no-op assert argument checks with production-active Preconditions validation, and redacts PrivateKey.toString() so the raw key can no longer leak through logs (explicit export moved to toStringWithPrivateKey()).

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

  • After the assertPreconditions change, the following arguments go from "silently accepted" to "throwing" in production: Signature.decode requires exactly 65 bytes, PublicKey requires exactly 64 bytes, recoverFromSignature requires recId ≥ 0 and non-negative r/s, etc. This is the fail-fast behavior the audit asked for and is an expected behavior change.
  • PrivateKey.toString() now returns a redacted constant. Any code that relied on toString() to export/persist the raw private key should switch to toStringWithPrivateKey() (which returns hex without the 0x prefix).
  • PrivateKey.create(String) still requires 64 hex chars (an optional 0x prefix is allowed); calls that previously passed an invalid length by relying on the no-op assert will now fail explicitly.

…ation

ABI decoder (abi): lengths and offsets read from untrusted return data /
event logs are bounds-checked before use. decodeUintAsInt rejects values
above Integer.MAX_VALUE; dynamic bytes and array lengths are capped by
the remaining input before shifting or pre-allocation; slot and
parameter-window reads share a checkWindowBounds helper covering both
the constructor-reflection and runtime-TypeReference struct decoding
engines; offset arithmetic uses Math.multiplyExact. Malformed input now
fails fast with IllegalArgumentException instead of integer overflow,
StringIndexOutOfBoundsException, NegativeArraySizeException, or an
oversized memory pre-allocation. Covered by TypeDecoderDoSTest.

SECP256K1 utilities (utils): no-op assert argument checks are replaced
with production-active Preconditions validation, PrivateKey.toString()
is redacted (explicit export via toStringWithPrivateKey()), and
PrivateKey.create tolerates an optional 0x prefix.
@0xbigapple
0xbigapple requested a review from 317787106 July 21, 2026 09:09
… precedence

The array rejection in decodeArrayElements checks four conditions but
reported only the length; the message now includes length, offset and
input length. Parenthesize the 0x-prefix condition in PrivateKey.create
to make the evaluation order explicit.
@kuny0707
kuny0707 merged commit 6eb4a08 into tronprotocol:release_1.0.0 Jul 22, 2026
3 checks passed
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.

3 participants