From a7aba24bad8469c0a368d877f1bfd571bbda2db9 Mon Sep 17 00:00:00 2001 From: hlts2 Date: Mon, 22 Jun 2026 16:57:42 +0900 Subject: [PATCH] fix(kubernetes): prevent command injection in application remove The marketplace application name supplied by the user was interpolated directly into a shell command string passed to /bin/bash -c, allowing arbitrary command execution. Fetch the uninstall script over HTTP and pipe it to 'bash -s' via stdin instead, with the app name URL-escaped and only ever used as a URL path segment. Also validate the HTTP status before executing so a missing script is not piped to the shell. Add a table-driven test for getMarketplaceAppUninstallScript using an httptest server. Signed-off-by: hlts2 --- cmd/kubernetes/kubernetes_app_remove.go | 34 +++++++++++-- cmd/kubernetes/kubernetes_app_remove_test.go | 50 ++++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 cmd/kubernetes/kubernetes_app_remove_test.go diff --git a/cmd/kubernetes/kubernetes_app_remove.go b/cmd/kubernetes/kubernetes_app_remove.go index 5271a516..6c3a61f4 100644 --- a/cmd/kubernetes/kubernetes_app_remove.go +++ b/cmd/kubernetes/kubernetes_app_remove.go @@ -3,6 +3,9 @@ package kubernetes import ( "bytes" "fmt" + "io" + "net/http" + "net/url" "os" "os/exec" "strings" @@ -14,6 +17,8 @@ import ( "github.com/spf13/cobra" ) +const marketplaceBaseURL = "https://raw.githubusercontent.com/civo/kubernetes-marketplace/master" + var kubernetesAppRemoveCmd = &cobra.Command{ Use: "remove", Example: "civo kubernetes application remove NAME --cluster CLUSTER_NAME", @@ -52,11 +57,19 @@ var kubernetesAppRemoveCmd = &cobra.Command{ defer os.Remove(tmpFile.Name()) for _, split := range allApps { appName := split + // TODO: Ideally this would come from the Civo API, but the Civo API doesn't currently return uninstall.sh // https://www.civo.com/api/kubernetes#listing-applications - filepath := fmt.Sprintf("bash <(curl -s https://raw.githubusercontent.com/civo/kubernetes-marketplace/master/%s/uninstall.sh)", appName) - cmdConfig := exec.Command("/bin/bash", "-c", filepath) + raw, err := getMarketplaceAppUninstallScript(marketplaceBaseURL, appName) + if err != nil { + utility.Error("Failed to get uninstall script: %v", err) + os.Exit(1) + } + + cmdConfig := exec.Command("/bin/bash", "-s") var b bytes.Buffer + + cmdConfig.Stdin = bytes.NewReader(raw) cmdConfig.Stdout = &b cmdConfig.Stderr = &b cmdConfig.Env = os.Environ() @@ -65,10 +78,8 @@ var kubernetesAppRemoveCmd = &cobra.Command{ if err := cmdConfig.Run(); err != nil { if exitError, ok := err.(*exec.ExitError); ok { utility.Error("Failed to uninstall application %s (exited with code %d)\n", appName, exitError.ExitCode()) - cmd := exec.Command("curl", "-s", fmt.Sprintf("https://raw.githubusercontent.com/civo/kubernetes-marketplace/master/%s/uninstall.sh", appName)) - output, _ := cmd.CombinedOutput() fmt.Println("--------------- Uninstall script ---------------") - fmt.Println(string(output)) + fmt.Println(string(raw)) fmt.Println("--------------- Uninstall output ---------------") if strings.Contains(b.String(), "command not found") { fmt.Printf("Uninstall script for application %s not found \n", appName) @@ -96,3 +107,16 @@ var kubernetesAppRemoveCmd = &cobra.Command{ }, } + +func getMarketplaceAppUninstallScript(baseURL, appName string) ([]byte, error) { + scriptURL := fmt.Sprintf("%s/%s/uninstall.sh", baseURL, url.PathEscape(appName)) + resp, err := http.Get(scriptURL) + if err != nil { + return nil, fmt.Errorf("failed to fetch uninstall script for %s: %w", appName, err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("uninstall script for application %s not found (HTTP %d)", appName, resp.StatusCode) + } + return io.ReadAll(resp.Body) +} diff --git a/cmd/kubernetes/kubernetes_app_remove_test.go b/cmd/kubernetes/kubernetes_app_remove_test.go new file mode 100644 index 00000000..5d3ae97a --- /dev/null +++ b/cmd/kubernetes/kubernetes_app_remove_test.go @@ -0,0 +1,50 @@ +package kubernetes + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestGetMarketplaceAppUninstallScript(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/metrics-server/uninstall.sh" { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("echo uninstalling")) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + tests := []struct { + name string + appName string + wantScript string + wantErr bool + }{ + { + name: "returns the script for an existing app", + appName: "metrics-server", + wantScript: "echo uninstalling", + wantErr: false, + }, + { + name: "returns an error for a missing app", + appName: "does-not-exist", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getMarketplaceAppUninstallScript(srv.URL, tt.appName) + if (err != nil) != tt.wantErr { + t.Fatalf("getMarketplaceAppUninstallScript(%q) error = %v, wantErr %v", tt.appName, err, tt.wantErr) + } + if !tt.wantErr && string(got) != tt.wantScript { + t.Errorf("got %q, want %q", string(got), tt.wantScript) + } + }) + } +}