Validate signing round 9 decommitments as curve points#7
Open
mswilkison wants to merge 1 commit into
Open
Conversation
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>
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
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.