From 51bbfa2d0848227ed446545aebb6ff79bcef5713 Mon Sep 17 00:00:00 2001
From: Leonor Oliveira <9090754+leonorfmartins@users.noreply.github.com>
Date: Wed, 26 Mar 2025 15:38:58 +0100
Subject: [PATCH] [Search] Fix CodeQL warnings (#101364)

* Fix CodeQL warnings

* Also validate if path is a file above the safe dir

* Lint

* [REVIEW] reduce possibility of exploits

* Comment

* Remove test scenario
---
 pkg/storage/unified/search/bleve.go      |  42 +++++++++-
 pkg/storage/unified/search/bleve_test.go | 102 +++++++++++++++++++++++
 2 files changed, 142 insertions(+), 2 deletions(-)

diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go
index e99df0870e3..0b6a2c496b3 100644
--- a/pkg/storage/unified/search/bleve.go
+++ b/pkg/storage/unified/search/bleve.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log/slog"
+	"math"
 	"os"
 	"path/filepath"
 	"slices"
@@ -173,6 +174,9 @@ func (b *bleveBackend) BuildIndex(ctx context.Context,
 			fname = b.start.Format("tmp-20060102-150405")
 		}
 		dir := filepath.Join(resourceDir, fname)
+		if !isValidPath(dir, b.opts.Root) {
+			b.log.Error("Directory is not valid", "directory", dir)
+		}
 		if resourceVersion > 0 {
 			info, _ := os.Stat(dir)
 			if info != nil && info.IsDir() {
@@ -263,6 +267,9 @@ func (b *bleveBackend) cleanOldIndexes(dir string, skip string) {
 	for _, file := range files {
 		if file.IsDir() && file.Name() != skip {
 			fpath := filepath.Join(dir, file.Name())
+			if !isValidPath(dir, b.opts.Root) {
+				b.log.Error("Path is not valid", "directory", fpath, "error", err)
+			}
 			err = os.RemoveAll(fpath)
 			if err != nil {
 				b.log.Error("Unable to remove old index folder", "directory", fpath, "error", err)
@@ -273,6 +280,21 @@ func (b *bleveBackend) cleanOldIndexes(dir string, skip string) {
 	}
 }
 
+// isValidPath does a sanity check in case it tries to access a different dir
+func isValidPath(path, safeDir string) bool {
+	if path == "" || safeDir == "" {
+		return false
+	}
+	cleanPath := filepath.Clean(path)
+	cleanSafeDir := filepath.Clean(safeDir)
+
+	rel, err := filepath.Rel(cleanSafeDir, cleanPath)
+	if err != nil {
+		return false
+	}
+	return !strings.HasPrefix(rel, "..") && !strings.Contains(rel, "\\")
+}
+
 // TotalDocs returns the total number of documents across all indices
 func (b *bleveBackend) TotalDocs() int64 {
 	var totalDocs int64
@@ -630,10 +652,19 @@ func (b *bleveIndex) toBleveSearchRequest(ctx context.Context, req *resource.Res
 		fields = append(fields, f)
 	}
 
+	size, err := safeInt64ToInt(req.Limit)
+	if err != nil {
+		return nil, resource.AsErrorResult(err)
+	}
+	offset, err := safeInt64ToInt(req.Offset)
+	if err != nil {
+		return nil, resource.AsErrorResult(err)
+	}
+
 	searchrequest := &bleve.SearchRequest{
 		Fields:  fields,
-		Size:    int(req.Limit),
-		From:    int(req.Offset),
+		Size:    size,
+		From:    offset,
 		Explain: req.Explain,
 		Facets:  facets,
 	}
@@ -768,6 +799,13 @@ func (b *bleveIndex) toBleveSearchRequest(ctx context.Context, req *resource.Res
 	return searchrequest, nil
 }
 
+func safeInt64ToInt(i64 int64) (int, error) {
+	if i64 > math.MaxInt32 || i64 < math.MinInt32 {
+		return 0, fmt.Errorf("int64 value %d overflows int", i64)
+	}
+	return int(i64), nil
+}
+
 func getSortFields(req *resource.ResourceSearchRequest) []string {
 	sorting := []string{}
 	for _, sort := range req.SortBy {
diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go
index 204a4a07cbd..d8d4af741ff 100644
--- a/pkg/storage/unified/search/bleve_test.go
+++ b/pkg/storage/unified/search/bleve_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math"
 	"os"
 	"path/filepath"
 	"testing"
@@ -596,3 +597,104 @@ func (nc StubAccessClient) Write(ctx context.Context, req *authzextv1.WriteReque
 func (nc StubAccessClient) BatchCheck(ctx context.Context, req *authzextv1.BatchCheckRequest) (*authzextv1.BatchCheckResponse, error) {
 	return nil, nil
 }
+
+func TestSafeInt64ToInt(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   int64
+		want    int
+		wantErr bool
+	}{
+		{
+			name:  "Valid int64 within int range",
+			input: 42,
+			want:  42,
+		},
+		{
+			name:    "Overflow int64 value",
+			input:   math.MaxInt64,
+			want:    0,
+			wantErr: true,
+		},
+		{
+			name:    "Underflow int64 value",
+			input:   math.MinInt64,
+			want:    0,
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := safeInt64ToInt(tt.input)
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func Test_isValidPath(t *testing.T) {
+	tests := []struct {
+		name    string
+		dir     string
+		safeDir string
+		want    bool
+	}{
+		{
+			name:    "valid path",
+			dir:     "/path/to/my-file/",
+			safeDir: "/path/to/",
+			want:    true,
+		},
+		{
+			name:    "valid path without trailing slash",
+			dir:     "/path/to/my-file",
+			safeDir: "/path/to",
+			want:    true,
+		},
+		{
+			name:    "path with double slashes",
+			dir:     "/path//to//my-file/",
+			safeDir: "/path/to/",
+			want:    true,
+		},
+		{
+			name:    "invalid path: ..",
+			dir:     "/path/../above/",
+			safeDir: "/path/to/",
+		},
+		{
+			name:    "invalid path: \\",
+			dir:     "\\path/to",
+			safeDir: "/path/to/",
+		},
+		{
+			name:    "invalid path: not under safe dir",
+			dir:     "/path/to.txt",
+			safeDir: "/path/to/",
+		},
+		{
+			name:    "invalid path: empty paths",
+			dir:     "",
+			safeDir: "/path/to/",
+		},
+		{
+			name:    "invalid path: different path",
+			dir:     "/other/path/to/my-file/",
+			safeDir: "/Some/other/path",
+		},
+		{
+			name:    "invalid path: empty safe path",
+			dir:     "/path/to/",
+			safeDir: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, isValidPath(tt.dir, tt.safeDir))
+		})
+	}
+}