btcec/schnorr: reject s >= group order in ParseSignature
What the developer wrote
Authored by Lrifton92
btcec/schnorr: reject s >= group order in ParseSignature
ParseSignature documents that it enforces the BIP-340 requirement that
the s component lie in the range [0, n-1], and the r component is
already checked against the field prime accordingly. However, the
overflow return value of s.SetByteSlice was discarded, so an s value
greater than or equal to the group order n was silently reduced modulo
n and accepted instead of being rejected.
This restores parity with the reference implementation in
decred/dcrd/dcrec/secp256k1/schnorr (of which this code is a port) and
with Bitcoin Core, both of which reject such encodings at parse time.
The ErrSigSTooBig error kind is already defined in the imported schnorr
package but was previously unused here.
A regression test covering the s == n and s > n encodings is added; it
fails before this change (the signatures are silently accepted) and
passes after.
Signed-off-by: Lrifton92 <Lrifton92@users.noreply.github.com>
What changed, and why it matters
This commit fixes a bug in how btcd parses Schnorr (BIP-340) digital signatures. The code was supposed to reject signatures whose 's' number was larger than the allowed group order, but instead it silently wrapped the value around (modulo reduction) and accepted it. This could let invalid signatures pass validation, potentially allowing signature malleability or unexpected behavior in systems relying on strict BIP-340 compliance. The fix checks the overflow flag and now rejects such signatures with the already-defined ErrSigSTooBig error.
Review callers of ParseSignature to determine whether any relied on the previous lenient behavior; ensure downstream validation logic treats ErrSigSTooBig as a hard failure. Consider auditing other scalar deserialization sites for similar ignored overflow returns.
Security signals we found
BIP-340 signature parsing non-compliance
silent modulo reduction of s component
signature malleability risk from non-canonical encodings
missing overflow check on scalar deserialization
restores parity with Bitcoin Core and decred/dcrd reference behavior
Evidence from the diff
In btcec/schnorr/signature.go, ParseSignature previously called s.SetByteSlice(sig[32:64]) and ignored the returned overflow boolean. SetByteSlice reduces the input modulo the group order n when it overflows, so any s >= n was silently accepted as s mod n. This violated BIP-340, which requires s in [0, n-1]. The patch captures the overflow return and returns signatureError(ecdsa_schnorr.ErrSigSTooBig, …) when overflow is true. A regression test is added covering r == p, s == n, and s > n cases.
Changed components
btcec/schnorr/signature.go:ParseSignaturebtcec/schnorr/signature_test.goInspect captured patch +42 / −1
diff --git a/btcec/schnorr/signature.go b/btcec/schnorr/signature.go
index fbfe161..05bc808 100644
--- a/btcec/schnorr/signature.go
+++ b/btcec/schnorr/signature.go
@@ -91,7 +91,10 @@ func ParseSignature(sig []byte) (*Signature, error) {
return nil, signatureError(ecdsa_schnorr.ErrSigRTooBig, str)
}
var s btcec.ModNScalar
- s.SetByteSlice(sig[32:64])
+ if overflow := s.SetByteSlice(sig[32:64]); overflow {
+ str := "invalid signature: s >= group order"
+ return nil, signatureError(ecdsa_schnorr.ErrSigSTooBig, str)
+ }
// Return the signature.
return NewSignature(&r, &s), nil
diff --git a/btcec/schnorr/signature_test.go b/btcec/schnorr/signature_test.go
index 9e99bbe..409c1f3 100644
--- a/btcec/schnorr/signature_test.go
+++ b/btcec/schnorr/signature_test.go
@@ -291,3 +291,41 @@ func TestSchnorrSignNoMutate(t *testing.T) {
t.Fatalf("private key modified: %v", err)
}
}
+
+// TestParseSignatureComponentRange ensures ParseSignature enforces the BIP-340
+// range restrictions on both the r and s components, in particular that an s
+// value greater than or equal to the group order n is rejected rather than
+// silently reduced modulo n.
+func TestParseSignatureComponentRange(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ sig string
+ err error
+ }{
+ {
+ name: "r == p",
+ sig: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f181522ec8eca07de4860a4acdd12909d831cc56cbbac4622082221a8768d1d09",
+ err: ecdsa_schnorr.ErrSigRTooBig,
+ },
+ {
+ name: "s == n",
+ sig: "4e45e16932b8af514961a1d3a1a25fdf3f4f7732e9d624c6c61548ab5fb8cd41fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141",
+ err: ecdsa_schnorr.ErrSigSTooBig,
+ },
+ {
+ name: "s > n",
+ sig: "4e45e16932b8af514961a1d3a1a25fdf3f4f7732e9d624c6c61548ab5fb8cd41fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364142",
+ err: ecdsa_schnorr.ErrSigSTooBig,
+ },
+ }
+
+ for _, test := range tests {
+ _, err := ParseSignature(decodeHex(test.sig))
+ if !errors.Is(err, test.err) {
+ t.Errorf("%s: mismatched err -- got %v, want %v",
+ test.name, err, test.err)
+ }
+ }
+}
Why this scored 64/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.