Make magnetar primitives total over malformed input

Four adversarial inputs each reached a crash or a wrong answer:

- thbsseReconstructGF judged evaluation points by raw value, so x == p
  (the reserved secret point 0 mod p) passed the zero guard and x, x+p
  read as two points instead of one; the same unreduced x also underflowed
  the Lagrange denominator (p + x_i - x_j wraps when x_j > p). Reduce every
  point mod p once and guard on the reduced value.
- VerifyForsThreshold never length-checked the public digest, so a short
  digest panicked inside forsPkFromSig. Refuse it with ErrForsShape, as
  the producing door already does.
- AggregateShareEnvelope indexed ReceivedShares[partyIndex-1]; a zero or
  out-of-range 1-based index wrapped to a huge uint and panicked. An
  unknown party aggregates to the zero row.
- slhVerify handed the zero id slhdsaIDForMode returns for an unknown mode
  straight to circl, which panics. An unrecognised mode is a failed
  verification, not a crash.

Each fix has a regression test in the area's tracked test file.

Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
2026-08-21 10:51:38 -07:00
co-authored by Hanzo Dev
parent e83eec6933
commit 84272319a4
8 changed files with 135 additions and 6 deletions
@@ -512,6 +512,13 @@ func VerifyForsThreshold(pub *ForsPublicMaterial, forsSig []byte) (bool, error)
if len(pub.PkSeed) != int(internal.n) {
return false, ErrForsShape
}
// The digest is read by forsPkFromSig to index k message blocks; too few bytes
// panics there. The producing door (DistributedForsSetup) refuses the same
// short shape, so the verifying door must too — a malformed public input is a
// refused verification, not a crash.
if len(pub.Digest) < int((internal.k*internal.a+7)/8) {
return false, ErrForsShape
}
adr := pub.Addr.forsAddrBuf()
got := make([]byte, internal.n)
forsPkFromSig(internal, got, pub.PkSeed, &adr, pub.Digest, forsSig)
@@ -202,3 +202,30 @@ func TestForsThreshold_CommitMismatch_Rejected(t *testing.T) {
t.Fatalf("expected ErrForsCommitMismatch for tampered share, got %v", err)
}
}
// TestVerifyForsThreshold_RefusesShortDigest pins that the verifying door
// length-checks the public digest before forsPkFromSig indexes it: a digest one
// byte short of ceil(k*a/8) is refused with ErrForsShape, not a panic.
func TestVerifyForsThreshold_RefusesShortDigest(t *testing.T) {
params := MustParamsFor(ModeM192s)
internal, ok := internalParamsForMode(ModeM192s)
if !ok {
t.Fatal("internal params for ModeM192s")
}
need := int((internal.k*internal.a + 7) / 8)
pub := &ForsPublicMaterial{
Params: params,
Digest: make([]byte, need-1),
AuthPaths: make([][]byte, internal.k),
PkFors: make([]byte, internal.n),
PkSeed: make([]byte, internal.n),
Threshold: 1,
}
ok2, err := VerifyForsThreshold(pub, make([]byte, internal.forsSigSize()))
if ok2 {
t.Fatal("a short digest must not verify")
}
if err != ErrForsShape {
t.Fatalf("short digest must be ErrForsShape, got %v", err)
}
}
+6
View File
@@ -1100,6 +1100,12 @@ func RunDKG(
func AggregateShareEnvelope(tr *PVSSTranscript, qualified map[uint32]struct{}, partyIndex uint32) []uint16 {
L := tr.Params.SeedSize
out := make([]uint16, L)
// partyIndex is 1-based (party p reads ReceivedShares[p-1]). A zero or
// out-of-range index would wrap partyIndex-1 to a huge uint and panic; an
// unknown party aggregates to the zero row, not a crash.
if partyIndex < 1 || int(partyIndex) > len(tr.ReceivedShares) {
return out
}
for i := 0; i < tr.Params.committeeSize(tr.Committee); i++ {
if _, ok := qualified[uint32(i+1)]; !ok {
continue
+19
View File
@@ -945,3 +945,22 @@ func TestPVSS_DKG_RaceClean(t *testing.T) {
t.Errorf("concurrent DKG run failed: %v", err)
}
}
// TestAggregateShareEnvelope_RefusesOutOfRangeParty pins that a 1-based party
// index out of range aggregates to the zero row rather than wrapping partyIndex-1
// to a huge uint and panicking.
func TestAggregateShareEnvelope_RefusesOutOfRangeParty(t *testing.T) {
params := MustParamsFor(ModeM192s)
tr := &PVSSTranscript{Params: params, Committee: make([]NodeID, 3)}
for _, idx := range []uint32{0, 99} {
row := AggregateShareEnvelope(tr, map[uint32]struct{}{}, idx)
if len(row) != params.SeedSize {
t.Fatalf("index %d: width %d, want %d", idx, len(row), params.SeedSize)
}
for b, v := range row {
if v != 0 {
t.Fatalf("index %d: out-of-range party must aggregate to zero, got %d at %d", idx, v, b)
}
}
}
}
+14 -6
View File
@@ -181,18 +181,26 @@ func thbsseReconstructGF(shares []thbsseShare, seedSize int) ([]uint16, error) {
if len(shares) < 1 {
return out, ErrNotEnoughShares
}
// Reduce every evaluation point mod p once, and guard on the reduced value.
// A raw x of 257 (== p) is the reserved secret point 0, and x and x+257 are
// one point presented twice; comparing raw values would admit both. The
// reduced points also keep the Lagrange denominator below from underflowing —
// (p + x_i - x_j) is positive only when x_j < p.
seen := make(map[uint32]struct{}, len(shares))
for _, s := range shares {
if s.X == 0 {
xs := make([]uint32, len(shares))
for idx, s := range shares {
xr := s.X % thbsseSharePrime
if xr == 0 {
return out, ErrZeroEvalPoint
}
if _, dup := seen[s.X]; dup {
if _, dup := seen[xr]; dup {
return out, ErrDuplicateEvalPoint
}
seen[s.X] = struct{}{}
seen[xr] = struct{}{}
if len(s.Y) != seedSize {
return out, ErrShareWireSize
}
xs[idx] = xr
}
t := len(shares)
@@ -206,9 +214,9 @@ func thbsseReconstructGF(shares []thbsseShare, seedSize int) ([]uint16, error) {
if i == j {
continue
}
negXj := thbsseSharePrime - (shares[j].X % thbsseSharePrime)
negXj := thbsseSharePrime - xs[j]
num = (num * negXj) % thbsseSharePrime
diff := (thbsseSharePrime + shares[i].X - shares[j].X) % thbsseSharePrime
diff := (thbsseSharePrime + xs[i] - xs[j]) % thbsseSharePrime
den = (den * diff) % thbsseSharePrime
}
denInv := thbsseModInvSmall(den, thbsseSharePrime)
+44
View File
@@ -698,3 +698,47 @@ func TestThbsSE_Combine_ResearchOnlyRuntimeBarrier(t *testing.T) {
t.Fatalf("Combine with ack should pass the barrier and fail on empty input, got %v", err)
}
}
// TestReconstructGF_ReservedAndAliasedEvalPoints pins that evaluation points are
// judged mod p: x == p is the reserved secret point 0 and is refused; x and x+p
// are one point and are refused as a duplicate; and an unreduced but distinct
// point reconstructs the same secret as its reduced form, so the Lagrange
// denominator does not underflow on an unreduced input.
func TestReconstructGF_ReservedAndAliasedEvalPoints(t *testing.T) {
secret := []uint16{9, 9, 9}
shares, err := thbsseDealRandomGF(secret, 5, 2, []byte("reserved-and-aliased-entropy-0123456789"))
if err != nil {
t.Fatalf("deal: %v", err)
}
if _, err := thbsseReconstructGF(
[]thbsseShare{{X: thbsseSharePrime, Y: shares[0].Y}, shares[1]}, len(secret),
); err != ErrZeroEvalPoint {
t.Fatalf("x == p must be ErrZeroEvalPoint, got %v", err)
}
dup := thbsseShare{X: shares[1].X + thbsseSharePrime, Y: shares[1].Y}
if _, err := thbsseReconstructGF(
[]thbsseShare{shares[1], dup}, len(secret),
); err != ErrDuplicateEvalPoint {
t.Fatalf("x and x+p must be ErrDuplicateEvalPoint, got %v", err)
}
unreduced := thbsseShare{X: shares[1].X + thbsseSharePrime, Y: shares[1].Y}
got, err := thbsseReconstructGF([]thbsseShare{shares[0], unreduced}, len(secret))
if err != nil {
t.Fatalf("unreduced-point reconstruct: %v", err)
}
want, err := thbsseReconstructGF([]thbsseShare{shares[0], shares[1]}, len(secret))
if err != nil {
t.Fatalf("reduced-point reconstruct: %v", err)
}
for i := range want {
if got[i] != want[i] {
t.Fatalf("unreduced point gave a different secret at %d: %d vs %d", i, got[i], want[i])
}
if want[i] != secret[i] {
t.Fatalf("reconstruct[%d] = %d, want the dealt %d", i, want[i], secret[i])
}
}
}
+6
View File
@@ -104,6 +104,12 @@ func VerifyCtx(params *Params, groupPubkey *PublicKey, message, ctx []byte, sig
// that touches the FIPS 205 implementation; centralising it makes
// the wire-identity dispatch auditable as a single function.
func slhVerify(id slhdsa.ID, packedPk, message, ctx, sig []byte) bool {
// id 0 is the sentinel slhdsaIDForMode returns for an unknown mode; it is not
// a scheme, and handing it to circl panics. A verifier must be total over its
// inputs — an unrecognised mode is a failed verification, not a crash.
if id == 0 {
return false
}
pk := slhdsa.PublicKey{ID: id}
if err := pk.UnmarshalBinary(packedPk); err != nil {
return false
+12
View File
@@ -817,3 +817,15 @@ func TestMagnetar_Wire_VerifyBytesCtx_RoundTrip(t *testing.T) {
t.Fatalf("VerifyBytesCtx accepted an over-long ctx")
}
}
// TestSlhVerify_TotalOverUnknownMode pins that verification is total over its
// mode input: an unknown mode resolves to the zero (invalid) id, and slhVerify
// under it returns false rather than handing a non-scheme to circl, which panics.
func TestSlhVerify_TotalOverUnknownMode(t *testing.T) {
if slhdsaIDForMode(Mode(0x7F)) != 0 {
t.Fatal("an unknown mode must resolve to the zero (invalid) id")
}
if slhVerify(slhdsaIDForMode(Mode(0x7F)), make([]byte, 48), nil, nil, make([]byte, 16224)) {
t.Fatal("verification under an unknown scheme must return false")
}
}