feat: block out of bounds symlinks (#9738)

cmd/argocd-repo-server/commands/argocd_repo_server.go

@@ -82,6 +82,7 @@ func NewCommand() *cobra.Command {
 		disableTLS                        bool
 		maxCombinedDirectoryManifestsSize string
 		cmpTarExcludedGlobs               []string
+		allowOutOfBoundsSymlinks          bool
 	)
 	var command = cobra.Command{
 		Use:               cliName,
@@ -123,6 +124,7 @@ func NewCommand() *cobra.Command {
 				SubmoduleEnabled:                             getSubmoduleEnabled(),
 				MaxCombinedDirectoryManifestsSize:            maxCombinedDirectoryManifestsQuantity,
 				CMPTarExcludedGlobs:                          cmpTarExcludedGlobs,
+				AllowOutOfBoundsSymlinks:                     allowOutOfBoundsSymlinks,
 			}, askPassServer)
 			errors.CheckError(err)
 
@@ -200,6 +202,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().BoolVar(&disableTLS, "disable-tls", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_DISABLE_TLS", false), "Disable TLS on the gRPC endpoint")
 	command.Flags().StringVar(&maxCombinedDirectoryManifestsSize, "max-combined-directory-manifests-size", env.StringFromEnv("ARGOCD_REPO_SERVER_MAX_COMBINED_DIRECTORY_MANIFESTS_SIZE", "10M"), "Max combined size of manifest files in a directory-type Application")
 	command.Flags().StringArrayVar(&cmpTarExcludedGlobs, "plugin-tar-exclude", env.StringsFromEnv("ARGOCD_REPO_SERVER_PLUGIN_TAR_EXCLUSIONS", []string{}, ";"), "Globs to filter when sending tarballs to plugins.")
+	command.Flags().BoolVar(&allowOutOfBoundsSymlinks, "allow-oob-symlinks", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS", false), "Allow out-of-bounds symlinks in repositories (not recommended)")
 
 	tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(&command)
 	cacheSrc = reposervercache.AddCacheFlagsToCmd(&command, func(client *redis.Client) {

docs/operator-manual/argocd-cmd-params-cm.yaml

@@ -113,3 +113,7 @@ data:
   reposerver.max.combined.directory.manifests.size: '10M'
   # Paths to be excluded from the tarball streamed to plugins. Separate with ;
   reposerver.plugin.tar.exclusions: ""
+  # Allow repositories to contain symlinks that leave the boundaries of the repository. 
+  # Changing this to "true" will not allow _all_ out-of-bounds symlinks. Those will still be blocked for things like values 
+  # files in Helm charts. But symlinks which are not explicitly blocked by other checks will be allowed.
+  reposerver.allow.oob.symlinks: "false"

docs/operator-manual/server-commands/argocd-repo-server.md

@@ -13,6 +13,7 @@ argocd-repo-server [flags]
 ### Options
 
 ```
+      --allow-oob-symlinks                             Allow out-of-bounds symlinks in repositories (not recommended)
       --default-cache-expiration duration              Cache expiration default (default 24h0m0s)
       --disable-tls                                    Disable TLS on the gRPC endpoint
   -h, --help                                           help for argocd-repo-server

docs/operator-manual/upgrading/2.4-2.5.md

@@ -1,5 +1,34 @@
 # v2.4 to 2.5
 
+## Out-of-bounds symlinks now blocked at fetch
+
+There have been several path traversal and identification vulnerabilities disclosed in the past related to symlinks. To help prevent any further vulnerabilities, we now scan all repositories and Helm charts for **out of bounds symlinks** at the time they are fetched and block further processing if they are found.
+
+An out-of-bounds symlink is defined as any symlink that leaves the root of the Git repository or Helm chart, even if the final target is within the root.
+
+If an out of bounds symlink is found, a warning will be printed to the repo server console and an error will be shown in the UI or CLI.
+
+Below is an example directory structure showing valid symlinks and invalid symlinks.
+
+```
+chart
+├── Chart.yaml
+├── values
+│   └── values.yaml
+├── bad-link.yaml   -> ../out-of-bounds.yaml       # Blocked
+├── bad-link-2.yaml -> ../chart/values/values.yaml # Blocked because it leaves the root
+├── bad-link-3.yaml -> /absolute/link.yaml         # Blocked
+└── good-link.yaml  -> values/values.yaml          # OK
+```
+
+If you rely on out of bounds symlinks, this check can be disabled one of three ways:
+
+1. The `--allow-oob-symlinks` argument on the repo server.
+2. The `reposerver.allow.oob.symlinks` key if you are using `argocd-cmd-params-cm`
+3. Directly setting `ARGOCD_REPO_SERVER_ALLOW_OOB_SYMLINKS` environment variable on the repo server.
+
+It is **strongly recommended** to leave this check enabled. Disabling the check will not allow _all_ out-of-bounds symlinks. Those will still be blocked for things like values files in Helm charts, but symlinks which are not explicitly blocked by other checks will be allowed.
+
 ## Upgraded Kustomize Version
 
 Note that bundled Kustomize version has been upgraded from 4.4.1 to 4.5.5.

manifests/base/repo-server/argocd-repo-server-deployment.yaml

@@ -113,6 +113,12 @@ spec:
                 name: argocd-cmd-params-cm
                 key: reposerver.plugin.tar.exclusions
                 optional: true
+          - name: ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS
+            valueFrom:
+              configMapKeyRef:
+                key: reposerver.allow.oob.symlinks
+                name: argocd-cmd-params-cm
+                optional: true
           - name: HELM_CACHE_HOME
             value: /helm-working-dir
           - name: HELM_CONFIG_HOME

manifests/core-install.yaml

@@ -9763,6 +9763,12 @@ spec:
               key: reposerver.plugin.tar.exclusions
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.allow.oob.symlinks
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME

manifests/ha/install.yaml

@@ -10870,6 +10870,12 @@ spec:
               key: reposerver.plugin.tar.exclusions
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.allow.oob.symlinks
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME

manifests/ha/namespace-install.yaml

@@ -1644,6 +1644,12 @@ spec:
               key: reposerver.plugin.tar.exclusions
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.allow.oob.symlinks
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME

manifests/install.yaml

@@ -10212,6 +10212,12 @@ spec:
               key: reposerver.plugin.tar.exclusions
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.allow.oob.symlinks
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME

manifests/namespace-install.yaml

@@ -986,6 +986,12 @@ spec:
               key: reposerver.plugin.tar.exclusions
               name: argocd-cmd-params-cm
               optional: true
+        - name: ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS
+          valueFrom:
+            configMapKeyRef:
+              key: reposerver.allow.oob.symlinks
+              name: argocd-cmd-params-cm
+              optional: true
         - name: HELM_CACHE_HOME
           value: /helm-working-dir
         - name: HELM_CONFIG_HOME

reposerver/repository/repository.go

@@ -104,6 +104,7 @@ type RepoServerInitConstants struct {
 	SubmoduleEnabled                             bool
 	MaxCombinedDirectoryManifestsSize            resource.Quantity
 	CMPTarExcludedGlobs                          []string
+	AllowOutOfBoundsSymlinks                     bool
 }
 
 // NewService returns a new instance of the Manifest service
@@ -318,6 +319,22 @@ func (s *Service) runRepoOperation(
 			return err
 		}
 		defer io.Close(closer)
+		if !s.initConstants.AllowOutOfBoundsSymlinks {
+			err := argopath.CheckOutOfBoundsSymlinks(chartPath)
+			if err != nil {
+				oobError := &argopath.OutOfBoundsSymlinkError{}
+				if errors.As(err, &oobError) {
+					log.WithFields(log.Fields{
+						"chart":    source.Chart,
+						"revision": revision,
+						"file":     oobError.File,
+					}).Warn("chart contains out-of-bounds symlink")
+					return fmt.Errorf("chart contains out-of-bounds symlinks. file: %s", oobError.File)
+				} else {
+					return err
+				}
+			}
+		}
 		return operation(chartPath, revision, revision, func() (*operationContext, error) {
 			return &operationContext{chartPath, ""}, nil
 		})
@@ -332,6 +349,23 @@ func (s *Service) runRepoOperation(
 
 		defer io.Close(closer)
 
+		if !s.initConstants.AllowOutOfBoundsSymlinks {
+			err := argopath.CheckOutOfBoundsSymlinks(gitClient.Root())
+			if err != nil {
+				oobError := &argopath.OutOfBoundsSymlinkError{}
+				if errors.As(err, &oobError) {
+					log.WithFields(log.Fields{
+						"repo":     repo.Repo,
+						"revision": revision,
+						"file":     oobError.File,
+					}).Warn("repository contains out-of-bounds symlink")
+					return fmt.Errorf("repository contains out-of-bounds symlinks. file: %s", oobError.File)
+				} else {
+					return err
+				}
+			}
+		}
+
 		commitSHA, err := gitClient.CommitSHA()
 		if err != nil {
 			return err
@@ -343,6 +377,7 @@ func (s *Service) runRepoOperation(
 				return err
 			}
 		}
+
 		// Here commitSHA refers to the SHA of the actual commit, whereas revision refers to the branch/tag name etc
 		// We use the commitSHA to generate manifests and store them in cache, and revision to retrieve them from cache
 		return operation(gitClient.Root(), commitSHA, revision, func() (*operationContext, error) {