🐛 fix-sanitize plugin manifest name in plugin-manager add to prevent p…

cmd/plugin-manager/add/add.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"syscall"
 
 	"github.com/konveyor/crane/internal/flags"
@@ -201,7 +202,25 @@ func (o *Options) run(args []string) error {
 	return nil
 }
 
+func validateFileInput(dir, filename string) error {
+	if filename == "" || filename == "." || filename == ".." || strings.ContainsRune(filename, '\\') || filename != filepath.Base(filename) {
+		return fmt.Errorf("invalid plugin name %q: must be a bare file name without path separators or traversal sequences", filename)
+	}
+
+	existingPath := filepath.Join(dir, filename)
+	if _, err := os.Lstat(existingPath); err == nil {
+		return fmt.Errorf("a plugin named %q already exists at %s, please remove it first before installing", filename, existingPath)
+	} else if !errors.Is(err, os.ErrNotExist) {
+		return fmt.Errorf("failed to check destination %s: %w", existingPath, err)
+	}
+	return nil
+}
+
 func downloadBinary(filepath string, filename string, url string, log *logrus.Logger) error {
+	if err := validateFileInput(filepath, filename); err != nil {
+		return err
+	}
+
 	var binaryContents io.Reader
 	isUrl, url := plugin.IsUrl(url)
 	if !isUrl {
@@ -229,7 +248,7 @@ func downloadBinary(filepath string, filename string, url string, log *logrus.Lo
 	}
 
 	// Create the file
-	pluginBinary, err := os.OpenFile(filepath+"/"+filename, syscall.O_RDWR|syscall.O_CREAT|syscall.O_TRUNC, 0777)
+	pluginBinary, err := os.OpenFile(filepath+"/"+filename, syscall.O_RDWR|syscall.O_CREAT|syscall.O_EXCL, 0755)
 	if err != nil {
 		return err
 	}

cmd/plugin-manager/add/add_test.go

@@ -0,0 +1,58 @@
+package add
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestValidateFileInput(t *testing.T) {
+	dir := t.TempDir()
+
+	tests := []struct {
+		name     string
+		filename string
+		wantErr  bool
+	}{
+		{name: "valid name", filename: "my-plugin", wantErr: false},
+		{name: "empty name", filename: "", wantErr: true},
+		{name: "dot", filename: ".", wantErr: true},
+		{name: "dotdot", filename: "..", wantErr: true},
+		{name: "forward slash", filename: "foo/bar", wantErr: true},
+		{name: "back slash", filename: "foo\\bar", wantErr: true},
+		{name: "traversal", filename: "../evil", wantErr: true},
+		{name: "absolute path", filename: "/etc/passwd", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := validateFileInput(dir, tt.filename); (err != nil) != tt.wantErr {
+				t.Errorf("validateFileInput(%q, %q) error = %v, wantErr %v", dir, tt.filename, err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestValidateFileInputExistingFile(t *testing.T) {
+	dir := t.TempDir()
+	filename := "existing-plugin"
+	if err := os.WriteFile(filepath.Join(dir, filename), []byte("x"), 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := validateFileInput(dir, filename); err == nil {
+		t.Errorf("expected error for existing file, got nil")
+	}
+}
+
+func TestValidateFileInputExistingSymlink(t *testing.T) {
+	dir := t.TempDir()
+	filename := "broken-link"
+	if err := os.Symlink(filepath.Join(dir, "does-not-exist"), filepath.Join(dir, filename)); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := validateFileInput(dir, filename); err == nil {
+		t.Errorf("expected error for existing broken symlink, got nil")
+	}
+}