Add validation to helm values file path to prevent potential dirtrav vulnerability

docs/content/en/docs-dev/user-guide/configuration-reference.md

@@ -314,7 +314,7 @@ One of `yamlField` or `regex` is required.
 | Field | Type | Description | Required |
 |-|-|-|-|
 | releaseName | string | The release name of helm deployment. By default, the release name is equal to the application name. | No |
-| valueFiles | []string | List of value files should be loaded. | No |
+| valueFiles | []string | List of value files should be loaded. Only local files stored under the application directory or remote files served at the http(s) endpoint are allowed. | No |
 | setFiles | map[string]string | List of file path for values. | No |
 | apiVersions | []string | Kubernetes api versions used for Capabilities.APIVersions. | No |
 | kubeVersion | string | Kubernetes version used for Capabilities.KubeVersion. | No |

pkg/app/piped/cloudprovider/kubernetes/helm.go

@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -30,6 +31,10 @@ import (
 	"github.com/pipe-cd/pipecd/pkg/config"
 )
 
+var (
+	allowedURLSchemes = []string{"http", "https"}
+)
+
 type Helm struct {
 	version  string
 	execPath string
@@ -63,6 +68,10 @@ func (c *Helm) TemplateLocalChart(ctx context.Context, appName, appDir, namespac
 
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
+			if err := verifyHelmValueFilePath(appDir, v); err != nil {
+				c.logger.Error("failed to verify values file path", zap.Error(err))
+				return "", err
+			}
 			args = append(args, "-f", v)
 		}
 		for k, v := range opts.SetFiles {
@@ -99,7 +108,7 @@ type helmRemoteGitChart struct {
 }
 
 func (c *Helm) TemplateRemoteGitChart(ctx context.Context, appName, appDir, namespace string, chart helmRemoteGitChart, gitClient gitClient, opts *config.InputHelmOptions) (string, error) {
-	// Firstly, we need to download the remote repositoy.
+	// Firstly, we need to download the remote repository.
 	repoDir, err := os.MkdirTemp("", "helm-remote-chart")
 	if err != nil {
 		return "", fmt.Errorf("unable to created temporary directory for storing remote helm chart: %w", err)
@@ -153,6 +162,10 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa
 
 	if opts != nil {
 		for _, v := range opts.ValueFiles {
+			if err := verifyHelmValueFilePath(appDir, v); err != nil {
+				c.logger.Error("failed to verify values file path", zap.Error(err))
+				return "", err
+			}
 			args = append(args, "-f", v)
 		}
 		for k, v := range opts.SetFiles {
@@ -199,3 +212,62 @@ func (c *Helm) TemplateRemoteChart(ctx context.Context, appName, appDir, namespa
 	}
 	return executor()
 }
+
+// verifyHelmValueFilePath verifies if the path of the values file references
+// a remote URL or inside the path where the application configuration file (i.e. *.pipecd.yaml) is located.
+func verifyHelmValueFilePath(appDir, valueFilePath string) error {
+	url, err := url.Parse(valueFilePath)
+	if err == nil && url.Scheme != "" {
+		for _, s := range allowedURLSchemes {
+			if strings.EqualFold(url.Scheme, s) {
+				return nil
+			}
+		}
+
+		return fmt.Errorf("scheme %s is not allowed to load values file", url.Scheme)
+	}
+
+	// valueFilePath is a path where non-default Helm values file is located.
+	if !filepath.IsAbs(valueFilePath) {
+		valueFilePath = filepath.Join(appDir, valueFilePath)
+	}
+
+	if isSymlink(valueFilePath) {
+		if valueFilePath, err = resolveSymlinkToAbsPath(valueFilePath, appDir); err != nil {
+			return err
+		}
+	}
+
+	// If a path outside of appDir is specified as the path for the values file,
+	// it may indicate that someone trying to illegally read a file as values file that
+	// exists in the environment where Piped is running.
+	if !strings.HasPrefix(valueFilePath, appDir) {
+		return fmt.Errorf("values file %s references outside the application configuration directory", valueFilePath)
+	}
+
+	return nil
+}
+
+// isSymlink returns the path is whether symbolic link or not.
+func isSymlink(path string) bool {
+	lstat, err := os.Lstat(path)
+	if err != nil {
+		return false
+	}
+
+	return lstat.Mode()&os.ModeSymlink == os.ModeSymlink
+}
+
+// resolveSymlinkToAbsPath resolves symbolic link to an absolute path.
+func resolveSymlinkToAbsPath(path, absParentDir string) (string, error) {
+	resolved, err := os.Readlink(path)
+	if err != nil {
+		return "", err
+	}
+
+	if !filepath.IsAbs(resolved) {
+		resolved = filepath.Join(absParentDir, resolved)
+	}
+
+	return resolved, nil
+}

pkg/app/piped/cloudprovider/kubernetes/helm_test.go

@@ -77,3 +77,92 @@ func TestTemplateLocalChart_WithNamespace(t *testing.T) {
 		require.Equal(t, namespace, metadata["namespace"])
 	}
 }
+
+func TestVerifyHelmValueFilePath(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		name          string
+		appDir        string
+		valueFilePath string
+		wantErr       bool
+	}{
+		{
+			name:          "Values file locates inside the app dir",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates inside the app dir (with ..)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "../../../testdata/testhelm/appconfdir/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates under the app dir",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "dir/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates under the app dir (with ..)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "../../../testdata/testhelm/appconfdir/dir/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "arbitrary file locates outside the app dir",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "/etc/hosts",
+			wantErr:       true,
+		},
+		{
+			name:          "arbitrary file locates outside the app dir (with ..)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "../../../../../../../../../../../../etc/hosts",
+			wantErr:       true,
+		},
+		{
+			name:          "Values file locates allowed remote URL (http)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "http://exmaple.com/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates allowed remote URL (https)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "https://exmaple.com/values.yaml",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file locates disallowed remote URL (ftp)",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "ftp://exmaple.com/values.yaml",
+			wantErr:       true,
+		},
+		{
+			name:          "Values file is symlink targeting valid values file",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "valid-symlink",
+			wantErr:       false,
+		},
+		{
+			name:          "Values file is symlink targeting invalid values file",
+			appDir:        "testdata/testhelm/appconfdir",
+			valueFilePath: "invalid-symlink",
+			wantErr:       true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := verifyHelmValueFilePath(tc.appDir, tc.valueFilePath)
+			if tc.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}

pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/invalid-symlink

@@ -0,0 +1 @@
+/etc/hosts

pkg/app/piped/cloudprovider/kubernetes/testdata/testhelm/appconfdir/valid-symlink

@@ -0,0 +1 @@
+dir/values.yaml