Enforce C2SP decimal integer rules and bounds across parsers

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)

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",

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",

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)
+		}
+	}
+}

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

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) {

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)

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),

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

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 {

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)
 			}
 

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/`,