Skip to content

Backport tBTC-relevant BNB v4 hardening#4

Open
mswilkison wants to merge 3 commits into
integrate-bnb-hardeningfrom
codex/bnb-332-tbtc-fixes
Open

Backport tBTC-relevant BNB v4 hardening#4
mswilkison wants to merge 3 commits into
integrate-bnb-hardeningfrom
codex/bnb-332-tbtc-fixes

Conversation

@mswilkison
Copy link
Copy Markdown

@mswilkison mswilkison commented May 21, 2026

Summary

Stacked on #2. This draft backports the BNB bnb-chain#332 fixes that map cleanly to Threshold's current ECDSA/tBTC path without taking BNB's v4 module-path churn or EdDSA/package cleanup.

Included:

  • Bound additional verifier-controlled response scalars before modular exponentiation:
    • DLN Alpha and nil input guards.
    • Threshold Paillier FactorProof W1, W2, Sigma, and V absolute bounds.
    • MtA/RangeProof exact upper-bound rejection and nil modulus guards.
  • Harden panic-DoS paths used by ECDSA signing/keygen:
    • ECPoint scalar multiplication returns nil on invalid inputs.
    • Schnorr proof verifiers reject invalid public points and zero/out-of-range scalars before scalar multiplication.
    • VSS share verification rejects nil, zero, and out-of-range shares/verifiers before scalar multiplication.
    • ECDSA signing round 4 rejects nil theta inverse.
    • ECDSA signing round 9 fixes the decommitment guard from && to ||.
  • Add mta.ErrRangeProofVerify and signing-round attribution text for peer range-proof rejection.
  • Reject non-2048-bit Paillier/NTilde values in ECDSA keygen round-1 message validation, matching the later round-2 contract.

Review follow-up:

  • Removed unused ScalarMultErr / ScalarBaseMultErr public API surface.
  • Confirmed the only production BobMid / BobMidWC callers are in ecdsa/signing/round_2.go, where mta.ErrRangeProofVerify attribution is now wired.
  • Added comments around the MtA and FactorProof response bounds to document why MtA uses exclusive upper bounds while Threshold FactorProof keeps its existing inclusive absolute-bound style.
  • The stricter DLN Alpha check is verifier-only tightening. Honest provers already produce Alpha in (1, N); this rejects malformed values that previously only passed after reduction modulo N.
  • Stabilized a pre-existing EdDSA keygen test flake exposed by CI by passing the reconstructed scalar to edwards.PrivKeyFromScalar as a fixed-width 32-byte buffer.
  • Pinned the existing DLN nil-proof-element rejection with a no-panic regression test for nil Alpha and nil T entries.
  • Made the ProofBobWC.Verify nil result check after xE.Add(pf.U) explicit.

Not included in this draft:

Validation

  • go test -count=1 ./crypto ./crypto/dlnproof ./crypto/mta ./crypto/paillier ./crypto/schnorr ./crypto/vss ./ecdsa/keygen ./ecdsa/signing
  • go test -count=1 ./ecdsa/resharing
  • go test -count=1 ./eddsa/keygen ./eddsa/signing ./eddsa/resharing
  • go test -count=1 ./crypto/mta ./ecdsa/signing after the final MtA nil-guard tweak
  • go test -count=1 -timeout 25m ./ecdsa/keygen after the review follow-up commit
  • go test -count=1 -timeout 25m ./eddsa/keygen after the CI scalar-width flake fix
  • go test -count=1 ./crypto/dlnproof ./crypto/mta after the Gemini follow-up commit
  • git diff --check

@mswilkison mswilkison force-pushed the codex/bnb-332-tbtc-fixes branch from 492bdd6 to c09f596 Compare May 21, 2026 17:08
mswilkison added a commit that referenced this pull request May 21, 2026
Doc + small defensive-code follow-ups to the PR #2/#4/#5 review thread.
None of these are security-blocking; they close out the cosmetic and
pre-existing items that did not warrant their own PR earlier.

- common/hash_utils.go: strengthen RejectionSample doc so the name does
  not mislead future readers — the function is modular reduction, not
  true rejection sampling, and the bias bound depends on q vs the hash
  width.
- crypto/paillier/factor_proof.go: document that the tagged and legacy
  FactorChallenge paths emit different challenge distributions
  (positive-only vs signed), and note that FactorVerify's CmpAbs bounds
  exist to accommodate the legacy signed encoding.
- tss/params.go: expand SetSessionNonceBytes docstring with the
  collision/uniqueness/entropy guidance reviewers asked for.
- ecdsa/{keygen,signing}/rounds.go: document the round-1-capture
  invariant for getSSID so future refactors do not silently drift the
  hashed round.number domain separator.
- crypto/ecpoint.go: flag SetCurve's in-place mutation in the doc
  comment; the chained-call style is a footgun on shared points.
- crypto/vss/feldman_vss.go: reject nil shares, nil/zero share IDs, and
  duplicate IDs in ReConstruct before the Lagrange loop, so malformed
  inputs return an error instead of panicking through ModInverse(0).
  Add focused test coverage for each rejection path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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