Merge pull request from GHSA-63qx-x74g-jcr7

Signed-off-by: jannfis <[email protected]>

reposerver/repository/repository.go

@@ -52,7 +52,6 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/io"
 	"github.com/argoproj/argo-cd/v2/util/ksonnet"
 	"github.com/argoproj/argo-cd/v2/util/kustomize"
-	"github.com/argoproj/argo-cd/v2/util/security"
 	"github.com/argoproj/argo-cd/v2/util/text"
 )
 
@@ -66,6 +65,9 @@ const (
 	ociPrefix                      = "oci://"
 )
 
+// List of protocol schemes allowed for fetching remote value files
+var allowedHelmRemoteProtocols = []string{"http", "https"}
+
 // Service implements ManifestService interface
 type Service struct {
 	repoLock                  *repositoryLock
@@ -554,6 +556,146 @@ func runHelmBuild(appPath string, h helm.Helm) error {
 	return ioutil.WriteFile(markerFile, []byte("marker"), 0644)
 }
 
+// resolveSymbolicLinkRecursive resolves the symlink path recursively to its
+// canonical path on the file system, with a maximum nesting level of maxDepth.
+// If path is not a symlink, returns the verbatim copy of path and err of nil.
+func resolveSymbolicLinkRecursive(path string, maxDepth int) (string, error) {
+	resolved, err := os.Readlink(path)
+	if err != nil {
+		// path is not a symbolic link
+		_, ok := err.(*os.PathError)
+		if ok {
+			return path, nil
+		}
+		// Other error has occured
+		return "", err
+	}
+
+	if maxDepth == 0 {
+		return "", fmt.Errorf("maximum nesting level reached")
+	}
+
+	return resolveSymbolicLinkRecursive(resolved, maxDepth-1)
+}
+
+// isURLSchemeAllowed returns true if the protocol scheme is in the list of
+// allowed URL schemes.
+func isURLSchemeAllowed(scheme string, allowed []string) bool {
+	isAllowed := false
+	if len(allowed) > 0 {
+		for _, s := range allowed {
+			if strings.EqualFold(scheme, s) {
+				isAllowed = true
+				break
+			}
+		}
+	}
+
+	// Empty scheme means local file
+	return isAllowed && scheme != ""
+}
+
+// resolveHelmValueFilePath will inspect and resolve a path to a Helm value
+// file, and make sure that its final path is within the boundaries of the
+// path specified in repoRoot.
+//
+// appPath is the path we're operating in, e.g. where a Helm chart was unpacked
+// to. repoRoot is the path to the root of the repository.
+//
+// If either appPath or repoRoot is relative, it will be treated as relative
+// to the current working directory.
+//
+// valueFile is the path to a value file, relative to appPath. If valueFile is
+// specified as an absolute path (i.e. leading slash), it will be treated as
+// relative to the repoRoot. In case valueFile is a symlink in the extracted
+// chart, it will be resolved recursively and the decision of whether it is in
+// the boundary of repoRoot will be made using the final resolved path.
+// valueFile can also be a remote URL with a protocol scheme as prefix,
+// in which case the scheme must be included in the list of allowed schemes
+// specified by allowedURLSchemes.
+//
+// Will return an error if either valueFile is outside the boundaries of the
+// repoRoot, valueFile is an URL with a forbidden protocol scheme or if
+// valueFile is a recursive symlink nested too deep. May return errors for
+// other reasons as well.
+//
+// resolvedPath will hold the absolute, resolved path for valueFile on success
+// or set to the empty string on failure.
+//
+// isRemote will be set to true if valueFile is an URL using an allowed
+// protocol scheme, or to false if it resolved to a local file.
+func resolveHelmValueFilePath(appPath, repoRoot, valueFile string, allowedURLSchemes []string) (resolvedPath string, isRemote bool, err error) {
+
+	// We do not provide the path in the error message, because it will be
+	// returned to the user and could be used for information gathering.
+	// Instead, we log the concrete error details.
+	resolveFailure := func(path string, err error) error {
+		log.Errorf("failed to resolve path '%s': %v", path, err)
+		return fmt.Errorf("internal error: failed to resolve path. Check logs for more details")
+	}
+
+	// A value file can be specified as an URL to a remote resource.
+	// We only allow certain URL schemes for remote value files.
+	url, err := url.Parse(valueFile)
+	if err == nil {
+		// If scheme is empty, it means we parsed a path only
+		if url.Scheme != "" {
+			if isURLSchemeAllowed(url.Scheme, allowedURLSchemes) {
+				return valueFile, true, nil
+			} else {
+				return "", false, fmt.Errorf("the URL scheme '%s' is not allowed", url.Scheme)
+			}
+		}
+	}
+
+	// Ensure that our repository root is absolute
+	absRepoPath, err := filepath.Abs(repoRoot)
+	if err != nil {
+		return "", false, resolveFailure(repoRoot, err)
+	}
+
+	// If the path to the file is relative, join it with the current working directory (appPath)
+	// Otherwise, join it with the repository's root
+	path := valueFile
+	if !filepath.IsAbs(path) {
+		absWorkDir, err := filepath.Abs(appPath)
+		if err != nil {
+			return "", false, resolveFailure(repoRoot, err)
+		}
+		path = filepath.Join(absWorkDir, path)
+	} else {
+		path = filepath.Join(absRepoPath, path)
+	}
+
+	// Ensure any symbolic link is resolved before we
+	delinkedPath, err := resolveSymbolicLinkRecursive(path, 10)
+	if err != nil {
+		return "", false, resolveFailure(path, err)
+	}
+	path = delinkedPath
+
+	// Resolve the joined path to an absolute path
+	path, err = filepath.Abs(path)
+	if err != nil {
+		return "", false, resolveFailure(path, err)
+	}
+
+	// Ensure our root path has a trailing slash, otherwise the following check
+	// would return true if root is /foo and path would be /foo2
+	requiredRootPath := absRepoPath
+	if !strings.HasSuffix(requiredRootPath, "/") {
+		requiredRootPath += "/"
+	}
+
+	// Make sure that the resolved path to values file is within the repository's root path
+	if !strings.HasPrefix(path, requiredRootPath) {
+		return "", false, fmt.Errorf("value file '%s' resolved to outside repository root", valueFile)
+	}
+
+	return path, false, nil
+
+}
+
 func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclient.ManifestRequest, isLocal bool) ([]*unstructured.Unstructured, error) {
 	concurrencyAllowed := isConcurrencyAllowed(appPath)
 	if !concurrencyAllowed {
@@ -583,31 +725,14 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
 		}
 
 		for _, val := range appHelm.ValueFiles {
-			// If val is not a URL, run it against the directory enforcer. If it is a URL, use it without checking
-			// If val does not exist, warn. If IgnoreMissingValueFiles, do not append, else let Helm handle it.
-			if _, err := url.ParseRequestURI(val); err != nil {
-
-				// Ensure that the repo root provided is absolute
-				absRepoPath, err := filepath.Abs(repoRoot)
-				if err != nil {
-					return nil, err
-				}
-
-				// If the path to the file is relative, join it with the current working directory (appPath)
-				path := val
-				if !filepath.IsAbs(path) {
-					absWorkDir, err := filepath.Abs(appPath)
-					if err != nil {
-						return nil, err
-					}
-					path = filepath.Join(absWorkDir, path)
-				}
 
-				_, err = security.EnforceToCurrentRoot(absRepoPath, path)
-				if err != nil {
-					return nil, err
-				}
+			// This will resolve val to an absolute path (or an URL)
+			path, isRemote, err := resolveHelmValueFilePath(appPath, repoRoot, val, allowedHelmRemoteProtocols)
+			if err != nil {
+				return nil, err
+			}
 
+			if !isRemote {
 				_, err = os.Stat(path)
 				if os.IsNotExist(err) {
 					if appHelm.IgnoreMissingValueFiles {
@@ -616,7 +741,8 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
 					}
 				}
 			}
-			templateOpts.Values = append(templateOpts.Values, val)
+
+			templateOpts.Values = append(templateOpts.Values, path)
 		}
 
 		if appHelm.Values != "" {