Validate signing round 9 decommitments as curve points#7
Merged
Conversation
piotr-roslaniec
force-pushed
the
codex/round9-decommit-validation
branch
2 times, most recently
from
edb23e5 to
1ed1a03
Compare
piotr-roslaniec
approved these changes
Jun 15, 2026
piotr-roslaniec
force-pushed
the
codex/review-residual-cleanup
branch
from
7e210dd to
20bb989
Compare
piotr-roslaniec
force-pushed
the
codex/round9-decommit-validation
branch
from
1ed1a03 to
627be04
Compare
piotr-roslaniec
force-pushed
the
codex/review-residual-cleanup
branch
from
20bb989 to
03554f7
Compare
Round 9 was the last place where adversarial wire data reached raw elliptic.Curve.Add without on-curve validation: a malformed U_j/T_j decommitment panics Go stdlib curves and yields undefined coordinates on btcec, and the resulting U != T abort blamed the honest reporting party itself. Validate the decommitted coordinates via crypto.NewECPoint, attribute failures to the sender, and report the unattributable U != T mismatch without a culprit. Also reject nil and negative messages in signing round 1: nil previously panicked on Cmp inside the protocol goroutine, and negative values only surfaced as an unattributed verification failure in finalize. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The block comments above the Fiat-Shamir challenge derivation claimed rejection sampling, but the code uses common.ModReduceHash (modular reduction). Describe the actual behaviour to avoid misleading auditors.
FactorVerify required s and t to be canonical generators but not distinct. With s == t the Pedersen-style commitment binding degenerates and a self-consistent proof over equal bases verifies as valid. Sibling proofs (dlnproof, MtA range/respondent) already reject equal generators; mirror that policy here as defense-in-depth. Add a regression test that builds a proof with s == t and confirms it is rejected (it verifies as true without the guard).
piotr-roslaniec
force-pushed
the
codex/round9-decommit-validation
branch
from
627be04 to
3af3ee0
Compare
piotr-roslaniec
changed the base branch from
codex/review-residual-cleanup
to
master
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #6.
Summary
Round 9 of ECDSA signing was the last place in the protocol where adversarial wire data reached raw
elliptic.Curve.Addwithout on-curve validation. The decommittedU_j/T_jcoordinates fromSignRound7Message/SignRound8Messagewent straight into group addition, while the analogous decommitments in rounds 5 and 7 are validated viacrypto.NewECPoint.tss.RegisterCurve) panic on off-curve inputs toAddsince Go 1.19, so a malicious decommitment was a remote crash vector. btcec returns undefined coordinates instead, turning the malformed input into aU != Tabort that blamed the honest reporting party itself (round.WrapError(..., round.PartyID())) — actively wrong fault attribution for orchestration layers that act on culprits.ECPoint.Addso an identity-summing decommitment (e.g.U_j = -U_i) is also rejected with sender attribution. The finalU != Tmismatch — which proves phase-5 misbehaviour but cannot identify the misbehaving party — is now reported without a culprit.mup front. Nil previously panicked onCmpinside the protocol goroutine (bypassingtss.Errorreporting), and a negative message only surfaced as an unattributed signature-verification failure in finalize.Upstream BNB
masterhas the same round-9 gap (plus the!ok && len(values) != 4decommit condition this fork already fixed), so there is no upstream commit to port; this is a residual finding from re-reviewing the hardened surface.Tests
TestRound9_RejectsMalformedDecommitments— off-curveU_j, off-curveT_j, and identity-summingU_jeach abort with the sender attributed (previously: panic on stdlib curves / self-blame on btcec).TestRound9_UTMismatchHasNoCulprit— on-curve but inconsistent decommitments abort with no culprit.TestRound9_ConsistentDecommitmentsSucceed— accept path still completes.TestSigning_Start_RejectsInvalidMessage— nil and negative messages failStartcleanly.go test ./...,go vet ./...,gofmt -lall clean.