diff --git a/log/checkpoint.go b/log/checkpoint.go index 97c4e65..cf6039c 100644 --- a/log/checkpoint.go +++ b/log/checkpoint.go @@ -64,6 +64,9 @@ func (c *Checkpoint) Unmarshal(data []byte) ([]byte, error) { if len(origin) == 0 { return nil, errors.New("invalid checkpoint - empty origin") } + if len(l[1]) > 1 && l[1][0] == '0' { + return nil, errors.New("invalid checkpoint - size has leading zeros") + } size, err := strconv.ParseUint(string(l[1]), 10, 64) if err != nil { return nil, fmt.Errorf("invalid checkpoint - size invalid: %w", err) diff --git a/log/checkpoint_test.go b/log/checkpoint_test.go index fda4d15..e60243c 100644 --- a/log/checkpoint_test.go +++ b/log/checkpoint_test.go @@ -71,6 +71,14 @@ func TestUnmarshalLogState(t *testing.T) { Size: 123, Hash: []byte("bananas"), }, + }, { + desc: "valid size zero", + m: "Log\n0\nYmFuYW5hcw==\n", + want: log.Checkpoint{ + Origin: "Log", + Size: 0, + Hash: []byte("bananas"), + }, }, { desc: "valid with different origin", m: "Banana\n9944\ndGhlIHZpZXcgZnJvbSB0aGUgdHJlZSB0b3BzIGlzIGdyZWF0IQ==\n", @@ -126,6 +134,18 @@ func TestUnmarshalLogState(t *testing.T) { desc: "invalid size - negative", m: "Log\n-34\ndGhlIHZpZXcgZnJvbSB0aGUgdHJlZSB0b3BzIGlzIGdyZWF0IQ==\n", wantErr: true, + }, { + desc: "invalid size - leading zero (00)", + m: "Log\n00\ndGhlIHZpZXcgZnJvbSB0aGUgdHJlZSB0b3BzIGlzIGdyZWF0IQ==\n", + wantErr: true, + }, { + desc: "invalid size - leading zero (0123)", + m: "Log\n0123\ndGhlIHZpZXcgZnJvbSB0aGUgdHJlZSB0b3BzIGlzIGdyZWF0IQ==\n", + wantErr: true, + }, { + desc: "invalid size - plus sign", + m: "Log\n+123\ndGhlIHZpZXcgZnJvbSB0aGUgdHJlZSB0b3BzIGlzIGdyZWF0IQ==\n", + wantErr: true, }, { desc: "invalid size - too large", m: "Log\n3438945738945739845734895735\ndGhlIHZpZXcgZnJvbSB0aGUgdHJlZSB0b3BzIGlzIGdyZWF0IQ==\n", diff --git a/log/note_test.go b/log/note_test.go index f61f5b1..c3922fc 100644 --- a/log/note_test.go +++ b/log/note_test.go @@ -107,6 +107,14 @@ func TestParseCheckpoint(t *testing.T) { wantErr: true, wantSigs: 1, }, + { + desc: "leading zero in size", + logID: "TestParseCheckpoint", + noteBody: "TestParseCheckpoint\n042\nYWJjZGVm\n", + sigs: []note.Signer{lns}, + wantErr: true, + wantSigs: 1, + }, { desc: "log and known1 sig", logID: "TestParseCheckpoint", diff --git a/note/note_cosigv1_test.go b/note/note_cosigv1_test.go index 9276503..33c3d5c 100644 --- a/note/note_cosigv1_test.go +++ b/note/note_cosigv1_test.go @@ -907,3 +907,28 @@ func TestCosigV1_RejectCRLF(t *testing.T) { } } } + +func TestMLDSACosigV1_RejectLeadingZeroSize(t *testing.T) { + mlSk, _ := mustGenerateMLDSAKey(t, "mldsa") + mlSigner, err := NewMLDSASigner(mlSk) + if err != nil { + t.Fatal(err) + } + validMsg := []byte("test\n123\nf+7CoKgXKE/tNys9TTXcr/ad6U/K3xvznmzew9y6SP0=\n") + sig, err := mlSigner.Sign(validMsg) + if err != nil { + t.Fatalf("Sign(validMsg): %v", err) + } + for _, badMsg := range [][]byte{ + []byte("test\n00\nf+7CoKgXKE/tNys9TTXcr/ad6U/K3xvznmzew9y6SP0=\n"), + []byte("test\n0123\nf+7CoKgXKE/tNys9TTXcr/ad6U/K3xvznmzew9y6SP0=\n"), + []byte("test\n+123\nf+7CoKgXKE/tNys9TTXcr/ad6U/K3xvznmzew9y6SP0=\n"), + } { + if _, err := mlSigner.Sign(badMsg); err == nil { + t.Errorf("Sign(%q) succeeded, want error", badMsg) + } + if mlSigner.Verifier().Verify(badMsg, sig) { + t.Errorf("Verifier().Verify(%q) = true, want false", badMsg) + } + } +} diff --git a/note/note_rfc6962.go b/note/note_rfc6962.go index bb48036..3a3ce80 100644 --- a/note/note_rfc6962.go +++ b/note/note_rfc6962.go @@ -241,6 +241,9 @@ func formatRFC6962STH(t uint64, msg []byte) (string, []byte, error) { if len(lines[3]) != 0 { return "", nil, errors.New("extension line(s) present") } + if len(lines[1]) > 1 && lines[1][0] == '0' { + return "", nil, errors.New("size has leading zeros") + } size, err := strconv.ParseUint(lines[1], 10, 64) if err != nil { return "", nil, err diff --git a/note/note_rfc6962_test.go b/note/note_rfc6962_test.go index bd63264..2871d97 100644 --- a/note/note_rfc6962_test.go +++ b/note/note_rfc6962_test.go @@ -105,6 +105,11 @@ func TestVerify(t *testing.T) { cp: []byte("B0rked" + romeCP), verifier: "rome.ct.filippo.io/2024h1+78f4abae+BTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAFzPC3fap+uINc1RQ4eRbYMUt84+bKkA8GDLN8KwVdzAgYhYSv4kS8XSheGLAHCWhIJTJbuC3sL88bNMTtrsBM=", wantErr: true, + }, { + name: "leading zero in size", + cp: []byte(strings.Replace(romeCP, "\n115474666\n", "\n0115474666\n", 1)), + verifier: "rome.ct.filippo.io/2024h1+78f4abae+BTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAFzPC3fap+uINc1RQ4eRbYMUt84+bKkA8GDLN8KwVdzAgYhYSv4kS8XSheGLAHCWhIJTJbuC3sL88bNMTtrsBM=", + wantErr: true, }, } { t.Run(test.name, func(t *testing.T) { diff --git a/policy/tlog_policy.go b/policy/tlog_policy.go index aa8409d..73e327b 100644 --- a/policy/tlog_policy.go +++ b/policy/tlog_policy.go @@ -232,6 +232,9 @@ func (p *TLogPolicy) Unmarshal(data []byte) error { case "all": k = len(members) default: + if len(threshold) > 1 && threshold[0] == '0' { + return fmt.Errorf("invalid threshold %q for group %q: leading zeros not allowed", threshold, name) + } u, err := strconv.ParseUint(threshold, 10, 31) if err != nil { return fmt.Errorf("invalid threshold %q for group %q: %w", threshold, name, err) diff --git a/policy/tlog_policy_test.go b/policy/tlog_policy_test.go index a318cd9..b20095b 100644 --- a/policy/tlog_policy_test.go +++ b/policy/tlog_policy_test.go @@ -341,6 +341,16 @@ func TestUnmarshal_Errors(t *testing.T) { policy: fmt.Sprintf("witness w1 %s\ngroup g1 -1 w1\nquorum g1\n", wit1_vkey), errStr: "invalid threshold", }, + { + desc: "threshold with leading zero", + policy: fmt.Sprintf("witness w1 %s\ngroup g1 01 w1\nquorum g1\n", wit1_vkey), + errStr: "leading zeros", + }, + { + desc: "zero threshold with leading zero", + policy: fmt.Sprintf("witness w1 %s\ngroup g1 00 w1\nquorum g1\n", wit1_vkey), + errStr: "leading zeros", + }, { desc: "threshold exceeds members", policy: fmt.Sprintf("witness w1 %s\ngroup g1 2 w1\nquorum g1\n", wit1_vkey), diff --git a/proof/tlog_proof.go b/proof/tlog_proof.go index ecb75e4..837c112 100644 --- a/proof/tlog_proof.go +++ b/proof/tlog_proof.go @@ -83,9 +83,12 @@ func (p *TLogProof) Unmarshal(data []byte) error { if !ok { return fmt.Errorf("tlog proof missing required index") } - idx, err = strconv.ParseUint(idxStr, 10, 64) + if len(idxStr) > 1 && idxStr[0] == '0' { + return fmt.Errorf("tlog proof index has leading zeros: %q", idxStr) + } + idx, err = strconv.ParseUint(idxStr, 10, 63) if err != nil { - return fmt.Errorf("tlog proof index not a valid uint64: %w", err) + return fmt.Errorf("tlog proof index not a valid int64: %w", err) } var hashes [][sha256.Size]byte diff --git a/proof/tlog_proof_test.go b/proof/tlog_proof_test.go index 0a06755..46102fe 100644 --- a/proof/tlog_proof_test.go +++ b/proof/tlog_proof_test.go @@ -19,6 +19,7 @@ import ( "crypto/sha256" "encoding/base64" "fmt" + "math" "strings" "testing" ) @@ -99,12 +100,37 @@ func TestUnmarshalErrors(t *testing.T) { { name: "invalid index - not a number", proof: []byte("c2sp.org/tlog-proof@v1\nindex notanumber\n\ncheckpoint\n"), - wantErrSubstr: "not a valid uint64", + wantErrSubstr: "not a valid", }, { name: "invalid index - negative", proof: []byte("c2sp.org/tlog-proof@v1\nindex -5\n\ncheckpoint\n"), - wantErrSubstr: "not a valid uint64", + wantErrSubstr: "not a valid", + }, + { + name: "invalid index - plus sign", + proof: []byte("c2sp.org/tlog-proof@v1\nindex +1\n\ncheckpoint\n"), + wantErrSubstr: "not a valid", + }, + { + name: "invalid index - leading zero (00)", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 00\n\ncheckpoint\n"), + wantErrSubstr: "leading zeros", + }, + { + name: "invalid index - leading zero (007)", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 007\n\ncheckpoint\n"), + wantErrSubstr: "leading zeros", + }, + { + name: "invalid index - 2^63", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 9223372036854775808\n\ncheckpoint\n"), + wantErrSubstr: "not a valid", + }, + { + name: "invalid index - MaxUint64", + proof: []byte("c2sp.org/tlog-proof@v1\nindex 18446744073709551615\n\ncheckpoint\n"), + wantErrSubstr: "not a valid", }, { name: "invalid hash base64", @@ -213,6 +239,14 @@ func TestRoundTrip(t *testing.T) { Checkpoint: []byte("checkpoint\n"), }, }, + { + name: "max index (2^63 - 1)", + proof: TLogProof{ + Index: math.MaxInt64, + Hashes: nil, + Checkpoint: []byte("checkpoint\n"), + }, + }, } for _, tt := range tests { diff --git a/witness/witness.go b/witness/witness.go index f9275dd..76407e9 100644 --- a/witness/witness.go +++ b/witness/witness.go @@ -93,7 +93,7 @@ func ParsePolicy(p []byte) (Group, error) { } components[name] = w case "group": - if len(fields) < 3 { + if len(fields) < 4 { return Group{}, fmt.Errorf("invalid group definition: %q", line) } @@ -111,13 +111,16 @@ func ParsePolicy(p []byte) (Group, error) { case "all": n = len(childrenNames) default: + if len(N) > 1 && N[0] == '0' { + return Group{}, fmt.Errorf("invalid threshold %q for group %q: leading zeros not allowed", N, name) + } i, err := strconv.ParseUint(N, 10, 8) if err != nil { return Group{}, fmt.Errorf("invalid threshold %q for group %q: %w", N, name, err) } n = int(i) } - if c := len(childrenNames); n > c { + if c := len(childrenNames); n < 1 || n > c { return Group{}, fmt.Errorf("group with %d children cannot have threshold %d", c, n) } diff --git a/witness/witness_policy_test.go b/witness/witness_policy_test.go index 23f3575..8fc0fc4 100644 --- a/witness/witness_policy_test.go +++ b/witness/witness_policy_test.go @@ -150,6 +150,27 @@ func TestParsePolicy_Errors(t *testing.T) { quorum g1`, errStr: "invalid threshold", }, + { + desc: "threshold with leading zero", + policy: `witness w1 sigsum.org+e4ade967+AZuUY6B08pW3QVHu8uvsrxWPcAv9nykap2Nb4oxCee+r https://sigsum.org/witness/ + group g1 01 w1 + quorum g1`, + errStr: "leading zeros", + }, + { + desc: "zero threshold", + policy: `witness w1 sigsum.org+e4ade967+AZuUY6B08pW3QVHu8uvsrxWPcAv9nykap2Nb4oxCee+r https://sigsum.org/witness/ + group g1 0 w1 + quorum g1`, + errStr: "cannot have threshold 0", + }, + { + desc: "group without members", + policy: `witness w1 sigsum.org+e4ade967+AZuUY6B08pW3QVHu8uvsrxWPcAv9nykap2Nb4oxCee+r https://sigsum.org/witness/ + group g1 all + quorum g1`, + errStr: "invalid group definition", + }, { desc: "witness name is keyword", policy: `witness all sigsum.org+e4ade967+AZuUY6B08pW3QVHu8uvsrxWPcAv9nykap2Nb4oxCee+r https://sigsum.org/witness/`,