(fix): gosec high severity issues

sampras343 · sampras343 · commit 947d35e39a7f · 2025-11-10T18:18:04.000Z
Signed-off-by: Sachin Sampras M &lt;sampras343@gmail.com&gt;
diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -150,7 +150,11 @@ func NewAPI(treeID int64) (*API, error) {
 		if !ok {
 			return nil, fmt.Errorf("no root found for inactive shard %d", r.TreeID)
 		}
-		cp, err := util.CreateAndSignCheckpoint(ctx, viper.GetString("rekor_server.hostname"), r.TreeID, uint64(r.TreeLength), root.RootHash, r.Signer)
+		treeLength, err := util.SafeInt64ToUint64(r.TreeLength)
+		if err != nil {
+			return nil, err
+		}
+		cp, err := util.CreateAndSignCheckpoint(ctx, viper.GetString("rekor_server.hostname"), r.TreeID, treeLength, root.RootHash, r.Signer)
 		if err != nil {
 			return nil, fmt.Errorf("error signing checkpoint for inactive shard %d: %w", r.TreeID, err)
 		}
diff --git a/pkg/api/entries.go b/pkg/api/entries.go
@@ -126,8 +126,13 @@ func logEntryFromLeaf(ctx context.Context, leaf *trillian.LogLeaf, signedLogRoot
 		sc = string(scBytes)
 	}
 
+	treeSize, err := util.SafeUint64ToInt64(root.TreeSize)
+	if err != nil {
+		return nil, err
+	}
+
 	inclusionProof := models.InclusionProof{
-		TreeSize:   conv.Pointer(int64(root.TreeSize)),
+		TreeSize:   conv.Pointer(treeSize),
 		RootHash:   conv.Pointer(hex.EncodeToString(root.RootHash)),
 		LogIndex:   conv.Pointer(proof.GetLeafIndex()),
 		Hashes:     hashes,
@@ -445,8 +450,13 @@ func createLogEntry(params entries.CreateLogEntryParams) (models.LogEntry, middl
 		return nil, handleRekorAPIError(params, http.StatusInternalServerError, err, sthGenerateError)
 	}
 
+	treeSize, err := util.SafeUint64ToInt64(root.TreeSize)
+	if err != nil {
+		return nil, handleRekorAPIError(params, http.StatusInternalServerError, err, validationError)
+	}
+
 	inclusionProof := models.InclusionProof{
-		TreeSize:   conv.Pointer(int64(root.TreeSize)),
+		TreeSize:   conv.Pointer(treeSize),
 		RootHash:   conv.Pointer(hex.EncodeToString(root.RootHash)),
 		LogIndex:   conv.Pointer(queuedLeaf.LeafIndex),
 		Hashes:     hashes,
diff --git a/pkg/api/tlog.go b/pkg/api/tlog.go
@@ -65,7 +65,10 @@ func GetLogInfoHandler(params tlog.GetLogInfoParams) middleware.Responder {
 	}
 
 	hashString := hex.EncodeToString(root.RootHash)
-	treeSize := int64(root.TreeSize)
+	treeSize, err := util.SafeUint64ToInt64(root.TreeSize)
+	if err != nil {
+		return handleRekorAPIError(params, http.StatusInternalServerError, err, validationError)
+	}
 
 	scBytes, err := util.CreateAndSignCheckpoint(ctx,
 		viper.GetString("rekor_server.hostname"), api.logRanges.GetActive().TreeID, root.TreeSize, root.RootHash, api.logRanges.GetActive().Signer)
@@ -164,7 +167,10 @@ func inactiveShardLogInfo(ctx context.Context, tid int64, cachedCheckpoints map[
 	}
 
 	hashString := hex.EncodeToString(root.RootHash)
-	treeSize := int64(root.TreeSize)
+	treeSize, err := util.SafeUint64ToInt64(root.TreeSize)
+	if err != nil {
+		return nil, err
+	}
 
 	m := models.InactiveShardLogInfo{
 		RootHash:       &hashString,
diff --git a/pkg/sharding/ranges.go b/pkg/sharding/ranges.go
@@ -31,6 +31,7 @@ import (
 	"github.com/sigstore/rekor/pkg/log"
 	"github.com/sigstore/rekor/pkg/signer"
 	"github.com/sigstore/rekor/pkg/trillianclient"
+	"github.com/sigstore/rekor/pkg/util"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"github.com/sigstore/sigstore/pkg/signature"
 	"github.com/sigstore/sigstore/pkg/signature/options"
@@ -116,7 +117,11 @@ func (l *LogRanges) CompleteInitialization(ctx context.Context, tcm *trilliancli
 		if err := root.UnmarshalBinary(resp.GetLatestResult.SignedLogRoot.LogRoot); err != nil {
 			return nil, err
 		}
-		l.inactive[i].TreeLength = int64(root.TreeSize)
+		treeSize, err := util.SafeUint64ToInt64(root.TreeSize)
+		if err != nil {
+			return nil, err
+		}
+		l.inactive[i].TreeLength = treeSize
 		sthMap[r.TreeID] = root
 	}
 	return sthMap, nil
diff --git a/pkg/trillianclient/trillian_client.go b/pkg/trillianclient/trillian_client.go
@@ -253,7 +253,15 @@ func (t *TrillianClient) GetLeafAndProofByIndex(ctx context.Context, index int64
 		})
 
 	if resp != nil && resp.Proof != nil {
-		if err := proof.VerifyInclusion(rfc6962.DefaultHasher, uint64(index), root.TreeSize, resp.GetLeaf().MerkleLeafHash, resp.Proof.Hashes, root.RootHash); err != nil {
+		u_index, err := util.SafeInt64ToUint64(index)
+		if err != nil {
+			return &Response{
+				Status: codes.OutOfRange,
+				Err:    status.Error(codes.OutOfRange, err.Error()),
+			}
+
+		}
+		if err := proof.VerifyInclusion(rfc6962.DefaultHasher, u_index, root.TreeSize, resp.GetLeaf().MerkleLeafHash, resp.Proof.Hashes, root.RootHash); err != nil {
 			return &Response{
 				Status: status.Code(err),
 				Err:    err,
diff --git a/pkg/util/safe_convertors.go b/pkg/util/safe_convertors.go
@@ -26,3 +26,11 @@ func SafeUint64ToInt64(u uint64) (int64, error) {
 	}
 	return int64(u), nil
 }
+
+
+func SafeInt64ToUint64(i int64) (uint64, error) {
+	if i < 0 {
+		return 0, fmt.Errorf("value %d is negative and cannot be converted to uint64", i)
+	}
+	return uint64(i), nil
+}
diff --git a/pkg/util/safe_convertors_test.go b/pkg/util/safe_convertors_test.go
@@ -15,16 +15,17 @@
 package util
 
 import (
+	"fmt"
 	"math"
 	"testing"
 )
 
 func TestSafeUint64ToInt64(t *testing.T) {
 	tests := []struct {
-		name      string
-		input     uint64
-		want      int64
-		wantErr   bool
+		name    string
+		input   uint64
+		want    int64
+		wantErr bool
 	}{
 		{
 			name:    "small positive number",
@@ -68,3 +69,62 @@ func TestSafeUint64ToInt64(t *testing.T) {
 		})
 	}
 }
+
+func TestSafeInt64ToUint64(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   int64
+		want    uint64
+		wantErr bool
+	}{
+		{
+			name:    "zero",
+			input:   0,
+			want:    0,
+			wantErr: false,
+		},
+		{
+			name:    "positive value",
+			input:   42,
+			want:    42,
+			wantErr: false,
+		},
+		{
+			name:    "max int64",
+			input:   math.MaxInt64,
+			want:    uint64(math.MaxInt64),
+			wantErr: false,
+		},
+		{
+			name:    "negative value",
+			input:   -1,
+			wantErr: true,
+		},
+		{
+			name:    "large negative",
+			input:   math.MinInt64,
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := SafeInt64ToUint64(tt.input)
+
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("SafeInt64ToUint64(%d) error = %v, wantErr %t", tt.input, err, tt.wantErr)
+			}
+
+			if !tt.wantErr && got != tt.want {
+				t.Fatalf("SafeInt64ToUint64(%d) = %d, want %d", tt.input, got, tt.want)
+			}
+
+			if tt.wantErr && err != nil {
+				expectedMsg := fmt.Sprintf("value %d is negative and cannot be converted to uint64", tt.input)
+				if err.Error() != expectedMsg {
+					t.Fatalf("error message = %q, want %q", err.Error(), expectedMsg)
+				}
+			}
+		})
+	}
+}
diff --git a/pkg/verify/verify.go b/pkg/verify/verify.go
@@ -63,7 +63,7 @@ func ProveConsistency(ctx context.Context, rClient *client.Rekor,
 	case oldTreeSize < newTreeSize:
 		consistencyParams := tlog.NewGetLogProofParamsWithContext(ctx)
 		consistencyParams.FirstSize = &oldTreeSize // Root size at the old, or trusted state.
-		consistencyParams.LastSize = newTreeSize // Root size at the new state to verify against.
+		consistencyParams.LastSize = newTreeSize   // Root size at the new state to verify against.
 		consistencyParams.TreeID = &treeID
 		consistencyProof, err := rClient.Tlog.GetLogProof(consistencyParams)
 		if err != nil {
@@ -174,8 +174,17 @@ func VerifyInclusion(ctx context.Context, e *models.LogEntryAnon) error {
 	}
 	leafHash := rfc6962.DefaultHasher.HashLeaf(entryBytes)
 
-	if err := proof.VerifyInclusion(rfc6962.DefaultHasher, uint64(*e.Verification.InclusionProof.LogIndex),
-		uint64(*e.Verification.InclusionProof.TreeSize), leafHash, hashes, rootHash); err != nil {
+	logIndex, err := util.SafeInt64ToUint64(*e.Verification.InclusionProof.LogIndex)
+	if err != nil {
+		return err
+	}
+	treeSize, err := util.SafeInt64ToUint64(*e.Verification.InclusionProof.TreeSize)
+	if err != nil {
+		return err
+	}
+
+	if err := proof.VerifyInclusion(rfc6962.DefaultHasher, logIndex,
+		treeSize, leafHash, hashes, rootHash); err != nil {
 		return err
 	}
 
diff --git a/pkg/verify/verify_test.go b/pkg/verify/verify_test.go
@@ -258,6 +258,46 @@ func TestInclusion(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "invalid inclusion - negative log index",
+			e: models.LogEntryAnon{
+				Body:           "eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoicmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiJlY2RjNTUzNmY3M2JkYWU4ODE2ZjBlYTQwNzI2ZWY1ZTliODEwZDkxNDQ5MzA3NTkwM2JiOTA2MjNkOTdiMWQ4In19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FWUNJUUQvUGRQUW1LV0MxKzBCTkVkNWdLdlFHcjF4eGwzaWVVZmZ2M2prMXp6Skt3SWhBTEJqM3hmQXlXeGx6NGpwb0lFSVYxVWZLOXZua1VVT1NvZVp4QlpQSEtQQyIsImZvcm1hdCI6Ing1MDkiLCJwdWJsaWNLZXkiOnsiY29udGVudCI6IkxTMHRMUzFDUlVkSlRpQlFWVUpNU1VNZ1MwVlpMUzB0TFMwS1RVWnJkMFYzV1VoTGIxcEplbW93UTBGUldVbExiMXBKZW1vd1JFRlJZMFJSWjBGRlRVOWpWR1pTUWxNNWFtbFlUVGd4UmxvNFoyMHZNU3R2YldWTmR3cHRiaTh6TkRjdk5UVTJaeTlzY21sVE56SjFUV2haT1V4alZDczFWVW8yWmtkQ1oyeHlOVm80VERCS1RsTjFZWE41WldRNVQzUmhVblozUFQwS0xTMHRMUzFGVGtRZ1VGVkNURWxESUV0RldTMHRMUzB0Q2c9PSJ9fX19",
+				IntegratedTime: &time,
+				LogID:          &logID,
+				LogIndex:       conv.Pointer(int64(-1)), // <- invalid
+				Verification: &models.LogEntryAnonVerification{
+					InclusionProof: &models.InclusionProof{
+						TreeSize: conv.Pointer(int64(2)),
+						RootHash: conv.Pointer("5be1758dd2228acfaf2546b4b6ce8aa40c82a3748f3dcb550e0d67ba34f02a45"),
+						LogIndex: conv.Pointer(int64(-1)), // <- invalid
+						Hashes: []string{
+							"59a575f157274702c38de3ab1e1784226f391fb79500ebf9f02b4439fb77574c",
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "invalid inclusion - negative tree size",
+			e: models.LogEntryAnon{
+				Body:           "eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoicmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiJlY2RjNTUzNmY3M2JkYWU4ODE2ZjBlYTQwNzI2ZWY1ZTliODEwZDkxNDQ5MzA3NTkwM2JiOTA2MjNkOTdiMWQ4In19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FWUNJUUQvUGRQUW1LV0MxKzBCTkVkNWdLdlFHcjF4eGwzaWVVZmZ2M2prMXp6Skt3SWhBTEJqM3hmQXlXeGx6NGpwb0lFSVYxVWZLOXZua1VVT1NvZVp4QlpQSEtQQyIsImZvcm1hdCI6Ing1MDkiLCJwdWJsaWNLZXkiOnsiY29udGVudCI6IkxTMHRMUzFDUlVkSlRpQlFWVUpNU1VNZ1MwVlpMUzB0TFMwS1RVWnJkMFYzV1VoTGIxcEplbW93UTBGUldVbExiMXBKZW1vd1JFRlJZMFJSWjBGRlRVOWpWR1pTUWxNNWFtbFlUVGd4UmxvNFoyMHZNU3R2YldWTmR3cHRiaTh6TkRjdk5UVTJaeTlzY21sVE56SjFUV2haT1V4alZDczFWVW8yWmtkQ1oyeHlOVm80VERCS1RsTjFZWE41WldRNVQzUmhVblozUFQwS0xTMHRMUzFGVGtRZ1VGVkNURWxESUV0RldTMHRMUzB0Q2c9PSJ9fX19",
+				IntegratedTime: &time,
+				LogID:          &logID,
+				LogIndex:       conv.Pointer(int64(1)),
+				Verification: &models.LogEntryAnonVerification{
+					InclusionProof: &models.InclusionProof{
+						TreeSize: conv.Pointer(int64(-10)), // <- invalid
+						RootHash: conv.Pointer("5be1758dd2228acfaf2546b4b6ce8aa40c82a3748f3dcb550e0d67ba34f02a45"),
+						LogIndex: conv.Pointer(int64(1)),
+						Hashes: []string{
+							"59a575f157274702c38de3ab1e1784226f391fb79500ebf9f02b4439fb77574c",
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	} {
 		t.Run(string(test.name), func(t *testing.T) {
 			ctx := context.Background()

Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,11 @@ func NewAPI(treeID int64) (*API, error) {`
`150`	`150`	`if !ok {`
`151`	`151`	`return nil, fmt.Errorf("no root found for inactive shard %d", r.TreeID)`
`152`	`152`	`}`
`153`		`- cp, err := util.CreateAndSignCheckpoint(ctx, viper.GetString("rekor_server.hostname"), r.TreeID, uint64(r.TreeLength), root.RootHash, r.Signer)`
	`153`	`+ treeLength, err := util.SafeInt64ToUint64(r.TreeLength)`
	`154`	`+ if err != nil {`
	`155`	`+ return nil, err`
	`156`	`+ }`
	`157`	`+ cp, err := util.CreateAndSignCheckpoint(ctx, viper.GetString("rekor_server.hostname"), r.TreeID, treeLength, root.RootHash, r.Signer)`
`154`	`158`	`if err != nil {`
`155`	`159`	`return nil, fmt.Errorf("error signing checkpoint for inactive shard %d: %w", r.TreeID, err)`
`156`	`160`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,3 +26,11 @@ func SafeUint64ToInt64(u uint64) (int64, error) {`
`26`	`26`	`}`
`27`	`27`	`return int64(u), nil`
`28`	`28`	`}`
	`29`	`+`
	`30`	`+`
	`31`	`+func SafeInt64ToUint64(i int64) (uint64, error) {`
	`32`	`+ if i < 0 {`
	`33`	`+ return 0, fmt.Errorf("value %d is negative and cannot be converted to uint64", i)`
	`34`	`+ }`
	`35`	`+ return uint64(i), nil`
	`36`	`+}`