From 5ea4e814f56be486efadfe69fb83f7171844ce9e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 14 Dec 2020 14:21:41 +0100 Subject: [PATCH] Add safe guards for relative paths This commit adds multiple safe guards for relative paths, ensuring they never traverse outside the working directory. The `SafeRelativePath` flag calculates the safe relative path based on a relative base dir, which results in a flattened path. The write methods of `manifestgen` make use of the `SecureJoin` as well, to ensure writes are never outside of the given directory when used as a lib outside of the CLI. Signed-off-by: Hidde Beydals --- cmd/flux/bootstrap.go | 29 +++++++------ cmd/flux/bootstrap_github.go | 21 ++++++---- cmd/flux/bootstrap_gitlab.go | 21 ++++++---- cmd/flux/create_kustomization.go | 8 ++-- docs/cmd/flux_bootstrap_github.md | 20 ++++----- docs/cmd/flux_bootstrap_gitlab.md | 18 ++++---- docs/cmd/flux_create_kustomization.md | 2 +- go.mod | 1 + go.sum | 9 +--- internal/flags/safe_relative_path.go | 50 +++++++++++++++++++++++ internal/flags/safe_relative_path_test.go | 49 ++++++++++++++++++++++ pkg/manifestgen/install/install.go | 7 +++- pkg/manifestgen/manifest.go | 16 +++++--- 13 files changed, 180 insertions(+), 71 deletions(-) create mode 100644 internal/flags/safe_relative_path.go create mode 100644 internal/flags/safe_relative_path_test.go diff --git a/cmd/flux/bootstrap.go b/cmd/flux/bootstrap.go index fd16af94..55b80e93 100644 --- a/cmd/flux/bootstrap.go +++ b/cmd/flux/bootstrap.go @@ -135,12 +135,11 @@ func generateInstallManifests(targetPath, namespace, tmpDir string, localManifes return "", fmt.Errorf("generating install manifests failed: %w", err) } - if filePath, err := output.WriteFile(tmpDir); err != nil { + filePath, err := output.WriteFile(tmpDir) + if err != nil { return "", fmt.Errorf("generating install manifests failed: %w", err) - } else { - return filePath, nil } - + return filePath, nil } func applyInstallManifests(ctx context.Context, manifestPath string, components []string) error { @@ -158,7 +157,7 @@ func applyInstallManifests(ctx context.Context, manifestPath string, components return nil } -func generateSyncManifests(url, branch, name, namespace, targetPath, tmpDir string, interval time.Duration) error { +func generateSyncManifests(url, branch, name, namespace, targetPath, tmpDir string, interval time.Duration) (string, error) { opts := sync.Options{ Name: name, Namespace: namespace, @@ -171,22 +170,22 @@ func generateSyncManifests(url, branch, name, namespace, targetPath, tmpDir stri manifest, err := sync.Generate(opts) if err != nil { - return fmt.Errorf("generating install manifests failed: %w", err) + return "", fmt.Errorf("generating install manifests failed: %w", err) } - if _, err := manifest.WriteFile(tmpDir); err != nil { - return err + output, err := manifest.WriteFile(tmpDir) + if err != nil { + return "", err } - - if err := utils.GenerateKustomizationYaml(filepath.Join(tmpDir, targetPath, namespace)); err != nil { - return err + outputDir := filepath.Dir(output) + if err := utils.GenerateKustomizationYaml(outputDir); err != nil { + return "", err } - - return nil + return outputDir, nil } -func applySyncManifests(ctx context.Context, kubeClient client.Client, name, namespace, targetPath, tmpDir string) error { - kubectlArgs := []string{"apply", "-k", filepath.Join(tmpDir, targetPath, namespace)} +func applySyncManifests(ctx context.Context, kubeClient client.Client, name, namespace, manifestsPath string) error { + kubectlArgs := []string{"apply", "-k", manifestsPath} if _, err := utils.ExecKubectlCommand(ctx, utils.ModeStderrOS, kubeconfig, kubecontext, kubectlArgs...); err != nil { return err } diff --git a/cmd/flux/bootstrap_github.go b/cmd/flux/bootstrap_github.go index e8c21606..f85e1ec1 100644 --- a/cmd/flux/bootstrap_github.go +++ b/cmd/flux/bootstrap_github.go @@ -29,8 +29,10 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/fluxcd/flux2/internal/utils" "github.com/fluxcd/pkg/git" + + "github.com/fluxcd/flux2/internal/flags" + "github.com/fluxcd/flux2/internal/utils" ) var bootstrapGitHubCmd = &cobra.Command{ @@ -75,7 +77,7 @@ var ( ghPersonal bool ghPrivate bool ghHostname string - ghPath string + ghPath flags.SafeRelativePath ghTeams []string ghDelete bool ghSSHHostname string @@ -94,7 +96,7 @@ func init() { bootstrapGitHubCmd.Flags().DurationVar(&ghInterval, "interval", time.Minute, "sync interval") bootstrapGitHubCmd.Flags().StringVar(&ghHostname, "hostname", git.GitHubDefaultHostname, "GitHub hostname") bootstrapGitHubCmd.Flags().StringVar(&ghSSHHostname, "ssh-hostname", "", "GitHub SSH hostname, to be used when the SSH host differs from the HTTPS one") - bootstrapGitHubCmd.Flags().StringVar(&ghPath, "path", "", "repository path, when specified the cluster sync will be scoped to this path") + bootstrapGitHubCmd.Flags().Var(&ghPath, "path", "path relative to the repository root, when specified the cluster sync will be scoped to this path") bootstrapGitHubCmd.Flags().BoolVar(&ghDelete, "delete", false, "delete repository (used for testing only)") bootstrapGitHubCmd.Flags().MarkHidden("delete") @@ -174,13 +176,13 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error { // generate install manifests logger.Generatef("generating manifests") - manifest, err := generateInstallManifests(ghPath, namespace, tmpDir, bootstrapManifestsPath) + installManifest, err := generateInstallManifests(ghPath.String(), namespace, tmpDir, bootstrapManifestsPath) if err != nil { return err } // stage install manifests - changed, err = repository.Commit(ctx, path.Join(ghPath, namespace), "Add manifests") + changed, err = repository.Commit(ctx, path.Join(ghPath.String(), namespace), "Add manifests") if err != nil { return err } @@ -206,7 +208,7 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error { if isInstall { // apply install manifests logger.Actionf("installing components in %s namespace", namespace) - if err := applyInstallManifests(ctx, manifest, bootstrapComponents()); err != nil { + if err := applyInstallManifests(ctx, installManifest, bootstrapComponents()); err != nil { return err } logger.Successf("install completed") @@ -259,12 +261,13 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error { // configure repo synchronization logger.Actionf("generating sync manifests") - if err := generateSyncManifests(repoURL, bootstrapBranch, namespace, namespace, ghPath, tmpDir, ghInterval); err != nil { + syncManifests, err := generateSyncManifests(repoURL, bootstrapBranch, namespace, namespace, ghPath.String(), tmpDir, ghInterval) + if err != nil { return err } // commit and push manifests - if changed, err = repository.Commit(ctx, path.Join(ghPath, namespace), "Add manifests"); err != nil { + if changed, err = repository.Commit(ctx, path.Join(ghPath.String(), namespace), "Add manifests"); err != nil { return err } else if changed { if err := repository.Push(ctx); err != nil { @@ -275,7 +278,7 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error { // apply manifests and waiting for sync logger.Actionf("applying sync manifests") - if err := applySyncManifests(ctx, kubeClient, namespace, namespace, ghPath, tmpDir); err != nil { + if err := applySyncManifests(ctx, kubeClient, namespace, namespace, syncManifests); err != nil { return err } diff --git a/cmd/flux/bootstrap_gitlab.go b/cmd/flux/bootstrap_gitlab.go index 1d8c6884..022c131f 100644 --- a/cmd/flux/bootstrap_gitlab.go +++ b/cmd/flux/bootstrap_gitlab.go @@ -29,8 +29,10 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/fluxcd/flux2/internal/utils" "github.com/fluxcd/pkg/git" + + "github.com/fluxcd/flux2/internal/flags" + "github.com/fluxcd/flux2/internal/utils" ) var bootstrapGitLabCmd = &cobra.Command{ @@ -73,7 +75,7 @@ var ( glPrivate bool glHostname string glSSHHostname string - glPath string + glPath flags.SafeRelativePath ) func init() { @@ -84,7 +86,7 @@ func init() { bootstrapGitLabCmd.Flags().DurationVar(&glInterval, "interval", time.Minute, "sync interval") bootstrapGitLabCmd.Flags().StringVar(&glHostname, "hostname", git.GitLabDefaultHostname, "GitLab hostname") bootstrapGitLabCmd.Flags().StringVar(&glSSHHostname, "ssh-hostname", "", "GitLab SSH hostname, to be used when the SSH host differs from the HTTPS one") - bootstrapGitLabCmd.Flags().StringVar(&glPath, "path", "", "repository path, when specified the cluster sync will be scoped to this path") + bootstrapGitLabCmd.Flags().Var(&glPath, "path", "path relative to the repository root, when specified the cluster sync will be scoped to this path") bootstrapCmd.AddCommand(bootstrapGitLabCmd) } @@ -145,13 +147,13 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error { // generate install manifests logger.Generatef("generating manifests") - manifest, err := generateInstallManifests(glPath, namespace, tmpDir, bootstrapManifestsPath) + installManifest, err := generateInstallManifests(glPath.String(), namespace, tmpDir, bootstrapManifestsPath) if err != nil { return err } // stage install manifests - changed, err = repository.Commit(ctx, path.Join(glPath, namespace), "Add manifests") + changed, err = repository.Commit(ctx, path.Join(glPath.String(), namespace), "Add manifests") if err != nil { return err } @@ -172,7 +174,7 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error { if isInstall { // apply install manifests logger.Actionf("installing components in %s namespace", namespace) - if err := applyInstallManifests(ctx, manifest, bootstrapComponents()); err != nil { + if err := applyInstallManifests(ctx, installManifest, bootstrapComponents()); err != nil { return err } logger.Successf("install completed") @@ -225,12 +227,13 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error { // configure repo synchronization logger.Actionf("generating sync manifests") - if err := generateSyncManifests(repoURL, bootstrapBranch, namespace, namespace, glPath, tmpDir, glInterval); err != nil { + syncManifests, err := generateSyncManifests(repoURL, bootstrapBranch, namespace, namespace, glPath.String(), tmpDir, glInterval) + if err != nil { return err } // commit and push manifests - if changed, err = repository.Commit(ctx, path.Join(glPath, namespace), "Add manifests"); err != nil { + if changed, err = repository.Commit(ctx, path.Join(glPath.String(), namespace), "Add manifests"); err != nil { return err } else if changed { if err := repository.Push(ctx); err != nil { @@ -241,7 +244,7 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error { // apply manifests and waiting for sync logger.Actionf("applying sync manifests") - if err := applySyncManifests(ctx, kubeClient, namespace, namespace, glPath, tmpDir); err != nil { + if err := applySyncManifests(ctx, kubeClient, namespace, namespace, syncManifests); err != nil { return err } diff --git a/cmd/flux/create_kustomization.go b/cmd/flux/create_kustomization.go index af9799c0..e355ed0e 100644 --- a/cmd/flux/create_kustomization.go +++ b/cmd/flux/create_kustomization.go @@ -74,7 +74,7 @@ var createKsCmd = &cobra.Command{ var ( ksSource flags.KustomizationSource - ksPath string + ksPath flags.SafeRelativePath = "./" ksPrune bool ksDependsOn []string ksValidation string @@ -88,7 +88,7 @@ var ( func init() { createKsCmd.Flags().Var(&ksSource, "source", ksSource.Description()) - createKsCmd.Flags().StringVar(&ksPath, "path", "./", "path to the directory containing a kustomization.yaml file") + createKsCmd.Flags().Var(&ksPath, "path", "path to the directory containing a kustomization.yaml file") createKsCmd.Flags().BoolVar(&ksPrune, "prune", false, "enable garbage collection") createKsCmd.Flags().StringArrayVar(&ksHealthCheck, "health-check", nil, "workload to be included in the health assessment, in the format '/.'") createKsCmd.Flags().DurationVar(&ksHealthTimeout, "health-check-timeout", 2*time.Minute, "timeout of health checking operations") @@ -110,7 +110,7 @@ func createKsCmdRun(cmd *cobra.Command, args []string) error { if ksPath == "" { return fmt.Errorf("path is required") } - if !strings.HasPrefix(ksPath, "./") { + if !strings.HasPrefix(ksPath.String(), "./") { return fmt.Errorf("path must begin with ./") } @@ -134,7 +134,7 @@ func createKsCmdRun(cmd *cobra.Command, args []string) error { Interval: metav1.Duration{ Duration: interval, }, - Path: ksPath, + Path: ksPath.String(), Prune: ksPrune, SourceRef: kustomizev1.CrossNamespaceSourceReference{ Kind: ksSource.Kind, diff --git a/docs/cmd/flux_bootstrap_github.md b/docs/cmd/flux_bootstrap_github.md index 124734f8..2836d346 100644 --- a/docs/cmd/flux_bootstrap_github.md +++ b/docs/cmd/flux_bootstrap_github.md @@ -46,16 +46,16 @@ flux bootstrap github [flags] ### Options ``` - -h, --help help for github - --hostname string GitHub hostname (default "github.com") - --interval duration sync interval (default 1m0s) - --owner string GitHub user or organization name - --path string repository path, when specified the cluster sync will be scoped to this path - --personal is personal repository - --private is private repository (default true) - --repository string GitHub repository name - --ssh-hostname string GitHub SSH hostname, to be used when the SSH host differs from the HTTPS one - --team stringArray GitHub team to be given maintainer access + -h, --help help for github + --hostname string GitHub hostname (default "github.com") + --interval duration sync interval (default 1m0s) + --owner string GitHub user or organization name + --path safeRelativePath path relative to the repository root, when specified the cluster sync will be scoped to this path + --personal is personal repository + --private is private repository (default true) + --repository string GitHub repository name + --ssh-hostname string GitHub SSH hostname, to be used when the SSH host differs from the HTTPS one + --team stringArray GitHub team to be given maintainer access ``` ### Options inherited from parent commands diff --git a/docs/cmd/flux_bootstrap_gitlab.md b/docs/cmd/flux_bootstrap_gitlab.md index 63c97ce5..399c8370 100644 --- a/docs/cmd/flux_bootstrap_gitlab.md +++ b/docs/cmd/flux_bootstrap_gitlab.md @@ -43,15 +43,15 @@ flux bootstrap gitlab [flags] ### Options ``` - -h, --help help for gitlab - --hostname string GitLab hostname (default "gitlab.com") - --interval duration sync interval (default 1m0s) - --owner string GitLab user or group name - --path string repository path, when specified the cluster sync will be scoped to this path - --personal is personal repository - --private is private repository (default true) - --repository string GitLab repository name - --ssh-hostname string GitLab SSH hostname, to be used when the SSH host differs from the HTTPS one + -h, --help help for gitlab + --hostname string GitLab hostname (default "gitlab.com") + --interval duration sync interval (default 1m0s) + --owner string GitLab user or group name + --path safeRelativePath path relative to the repository root, when specified the cluster sync will be scoped to this path + --personal is personal repository + --private is private repository (default true) + --repository string GitLab repository name + --ssh-hostname string GitLab SSH hostname, to be used when the SSH host differs from the HTTPS one ``` ### Options inherited from parent commands diff --git a/docs/cmd/flux_create_kustomization.md b/docs/cmd/flux_create_kustomization.md index 4d6466aa..804f06dd 100644 --- a/docs/cmd/flux_create_kustomization.md +++ b/docs/cmd/flux_create_kustomization.md @@ -50,7 +50,7 @@ flux create kustomization [name] [flags] --health-check stringArray workload to be included in the health assessment, in the format '/.' --health-check-timeout duration timeout of health checking operations (default 2m0s) -h, --help help for kustomization - --path string path to the directory containing a kustomization.yaml file (default "./") + --path safeRelativePath path to the directory containing a kustomization.yaml file (default ./) --prune enable garbage collection --service-account string the name of the service account to impersonate when reconciling this Kustomization --source kustomizationSource source that contains the Kubernetes manifests in the format '[/]',where kind can be one of: (GitRepository, Bucket), if kind is not specified it defaults to GitRepository diff --git a/go.mod b/go.mod index 9a78f405..c2044771 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.15 require ( github.com/blang/semver/v4 v4.0.0 + github.com/cyphar/filepath-securejoin v0.2.2 github.com/fluxcd/helm-controller/api v0.4.3 github.com/fluxcd/image-automation-controller/api v0.1.0 github.com/fluxcd/image-reflector-controller/api v0.1.0 diff --git a/go.sum b/go.sum index 4a13585c..a9d4d224 100644 --- a/go.sum +++ b/go.sum @@ -6,7 +6,6 @@ cloud.google.com/go v0.44.2/go.mod h1:60680Gw3Yr4ikxnPRS/oxxkBccT6SA1yMk63TGekxK cloud.google.com/go v0.45.1/go.mod h1:RpBamKRgapWJb87xiFSdk4g1CME7QZg3uwTez+TSTjc= cloud.google.com/go v0.46.3/go.mod h1:a6bKKbmY7er1mI7TEI4lsAkts/mkhTSZK8w33B4RAg0= cloud.google.com/go v0.50.0/go.mod h1:r9sluTvynVuxRIOHXQEHMFffphuXHOMZMycpNR5e6To= -cloud.google.com/go v0.51.0 h1:PvKAVQWCtlGUSlZkGW3QLelKaWq7KYv/MW1EboG8bfM= cloud.google.com/go v0.51.0/go.mod h1:hWtGJ6gnXH+KgDv+V0zFGDvpi07n3z8ZNj3T1RW0Gcw= cloud.google.com/go v0.52.0/go.mod h1:pXajvRH/6o3+F9jDHZWQ5PbGhn+o8w9qiu/CffaVdO4= cloud.google.com/go v0.53.0/go.mod h1:fp/UouUEsRkN6ryDKNW/Upv/JBKnv6WDthjR6+vze6M= @@ -33,24 +32,20 @@ github.com/Azure/go-autorest v14.2.0+incompatible h1:V5VMDjClD3GiElqLWO7mz2MxNAK github.com/Azure/go-autorest v14.2.0+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24= github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+BFDTQ8rEZB0s4Yfa6bI= github.com/Azure/go-autorest/autorest v0.9.3/go.mod h1:GsRuLYvwzLjjjRoWEIyMUaYq8GNUx2nRB378IPt/1p0= -github.com/Azure/go-autorest/autorest v0.9.6 h1:5YWtOnckcudzIw8lPPBcWOnmIFWMtHci1ZWAZulMSx0= github.com/Azure/go-autorest/autorest v0.9.6/go.mod h1:/FALq9T/kS7b5J5qsQ+RSTUdAmGFqi0vUdVNNx8q630= github.com/Azure/go-autorest/autorest v0.10.2 h1:NuSF3gXetiHyUbVdneJMEVyPUYAe5wh+aN08JYAf1tI= github.com/Azure/go-autorest/autorest v0.10.2/go.mod h1:/FALq9T/kS7b5J5qsQ+RSTUdAmGFqi0vUdVNNx8q630= github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0= github.com/Azure/go-autorest/autorest/adal v0.8.0/go.mod h1:Z6vX6WXXuyieHAXwMj0S6HY6e6wcHn37qQMBQlvY3lc= -github.com/Azure/go-autorest/autorest/adal v0.8.2 h1:O1X4oexUxnZCaEUGsvMnr8ZGj8HI37tNezwY4npRqA0= github.com/Azure/go-autorest/autorest/adal v0.8.2/go.mod h1:ZjhuQClTqx435SRJ2iMlOxPYt3d2C/T/7TiQCVZSn3Q= github.com/Azure/go-autorest/autorest/adal v0.9.5 h1:Y3bBUV4rTuxenJJs41HU3qmqsb+auo+a3Lz+PlJPpL0= github.com/Azure/go-autorest/autorest/adal v0.9.5/go.mod h1:B7KF7jKIeC9Mct5spmyCB/A8CG/sEz1vwIRGv/bbw7A= github.com/Azure/go-autorest/autorest/date v0.1.0/go.mod h1:plvfp3oPSKwf2DNjlBjWF/7vwR+cUD/ELuzDCXwHUVA= -github.com/Azure/go-autorest/autorest/date v0.2.0 h1:yW+Zlqf26583pE43KhfnhFcdmSWlm5Ew6bxipnr/tbM= github.com/Azure/go-autorest/autorest/date v0.2.0/go.mod h1:vcORJHLJEh643/Ioh9+vPmf1Ij9AEBM5FuBIXLmIy0g= github.com/Azure/go-autorest/autorest/date v0.3.0 h1:7gUk1U5M/CQbp9WoqinNzJar+8KY+LPI6wiWrP/myHw= github.com/Azure/go-autorest/autorest/date v0.3.0/go.mod h1:BI0uouVdmngYNUzGWeSYnokU+TrmwEsOqdt8Y6sso74= github.com/Azure/go-autorest/autorest/mocks v0.1.0/go.mod h1:OTyCOPRA2IgIlWxVYxBee2F5Gr4kF2zd2J5cFRaIDN0= github.com/Azure/go-autorest/autorest/mocks v0.2.0/go.mod h1:OTyCOPRA2IgIlWxVYxBee2F5Gr4kF2zd2J5cFRaIDN0= -github.com/Azure/go-autorest/autorest/mocks v0.3.0 h1:qJumjCaCudz+OcqE9/XtEPfvtOjOmKaui4EOpFI6zZc= github.com/Azure/go-autorest/autorest/mocks v0.3.0/go.mod h1:a8FDP3DYzQ4RYfVAxAN3SVSiiO77gL2j2ronKKP0syM= github.com/Azure/go-autorest/autorest/mocks v0.4.1 h1:K0laFcLE6VLTOwNgSxaGbUcLPuGXlNkbVvq4cW4nIHk= github.com/Azure/go-autorest/autorest/mocks v0.4.1/go.mod h1:LTp+uSrOhSkaKrUy935gNZuuIPPVsHlr9DSOxSayd+k= @@ -60,7 +55,6 @@ github.com/Azure/go-autorest/autorest/validation v0.1.0/go.mod h1:Ha3z/SqBeaalWQ github.com/Azure/go-autorest/autorest/validation v0.2.0/go.mod h1:3EEqHnBxQGHXRYq3HT1WyXAvT7LLY3tl70hw6tQIbjI= github.com/Azure/go-autorest/logger v0.1.0 h1:ruG4BSDXONFRrZZJ2GUXDiUyVpayPmb1GnWeHDdaNKY= github.com/Azure/go-autorest/logger v0.1.0/go.mod h1:oExouG+K6PryycPJfVSxi/koC6LSNgds39diKLz7Vrc= -github.com/Azure/go-autorest/tracing v0.5.0 h1:TRn4WjSnkcSy5AEG3pnbtFSwNtwzjr4VYyQflFE619k= github.com/Azure/go-autorest/tracing v0.5.0/go.mod h1:r/s2XiOKccPW3HrqB+W0TQzfbtp2fGCgRFtBroKn4Dk= github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo= github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= @@ -142,11 +136,12 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= +github.com/cyphar/filepath-securejoin v0.2.2 h1:jCwT2GTP+PY5nBz3c/YL5PAIbusElVrPujOBSCj8xRg= +github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4= github.com/davecgh/go-spew v0.0.0-20151105211317-5215b55f46b2/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E= diff --git a/internal/flags/safe_relative_path.go b/internal/flags/safe_relative_path.go new file mode 100644 index 00000000..b203674f --- /dev/null +++ b/internal/flags/safe_relative_path.go @@ -0,0 +1,50 @@ +/* +Copyright 2020 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flags + +import ( + "fmt" + "strings" + + securejoin "github.com/cyphar/filepath-securejoin" +) + +type SafeRelativePath string + +func (p *SafeRelativePath) String() string { + return string(*p) +} + +func (p *SafeRelativePath) Set(str string) error { + // The result of secure joining on a relative base dir is a flattened relative path. + cleanP, err := securejoin.SecureJoin("./", strings.TrimSpace(str)) + if err != nil { + return fmt.Errorf("invalid relative path '%s': %w", cleanP, err) + } + // NB: required, as a secure join of "./" will result in "." + cleanP = fmt.Sprintf("./%s", strings.TrimPrefix(cleanP, ".")) + *p = SafeRelativePath(cleanP) + return nil +} + +func (p *SafeRelativePath) Type() string { + return "safeRelativePath" +} + +func (p *SafeRelativePath) Description() string { + return fmt.Sprintf("secure relative path") +} diff --git a/internal/flags/safe_relative_path_test.go b/internal/flags/safe_relative_path_test.go new file mode 100644 index 00000000..872085ad --- /dev/null +++ b/internal/flags/safe_relative_path_test.go @@ -0,0 +1,49 @@ +/* +Copyright 2020 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flags + +import ( + "testing" +) + +func TestRelativePath_Set(t *testing.T) { + tests := []struct { + name string + str string + expect string + expectErr bool + }{ + {"relative path", "./foo", "./foo", false}, + {"relative path", "foo", "./foo", false}, + {"traversing relative path", "./foo/../bar", "./bar", false}, + {"absolute path", "/foo", "./foo", false}, + {"traversing absolute path", "/foo/../bar", "./bar", false}, + {"traversing overflowing absolute path", "/foo/../../../bar", "./bar", false}, + {"empty", "", "./", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var p SafeRelativePath + if err := p.Set(tt.str); (err != nil) != tt.expectErr { + t.Errorf("Set() error = %v, expectErr %v", err, tt.expectErr) + } + if str := p.String(); str != tt.expect { + t.Errorf("Set() = %v, expect %v", str, tt.expect) + } + }) + } +} diff --git a/pkg/manifestgen/install/install.go b/pkg/manifestgen/install/install.go index 4438219f..06a516bf 100644 --- a/pkg/manifestgen/install/install.go +++ b/pkg/manifestgen/install/install.go @@ -24,6 +24,8 @@ import ( "path" "strings" + securejoin "github.com/cyphar/filepath-securejoin" + "github.com/fluxcd/flux2/pkg/manifestgen" ) @@ -40,7 +42,10 @@ func Generate(options Options) (*manifestgen.Manifest, error) { } defer os.RemoveAll(tmpDir) - output := path.Join(tmpDir, options.ManifestFile) + output, err := securejoin.SecureJoin(tmpDir, options.ManifestFile) + if err != nil { + return nil, err + } if !strings.HasPrefix(options.BaseURL, "http") { if err := build(options.BaseURL, output); err != nil { diff --git a/pkg/manifestgen/manifest.go b/pkg/manifestgen/manifest.go index 049f3936..114c10a3 100644 --- a/pkg/manifestgen/manifest.go +++ b/pkg/manifestgen/manifest.go @@ -20,8 +20,9 @@ import ( "fmt" "io/ioutil" "os" - "path" "path/filepath" + + securejoin "github.com/cyphar/filepath-securejoin" ) // Manifest holds the data of a multi-doc YAML @@ -36,14 +37,17 @@ type Manifest struct { // If the file does not exist, WriteFile creates it with permissions perm, // otherwise WriteFile overwrites the file, without changing permissions. func (m *Manifest) WriteFile(rootDir string) (string, error) { - if err := os.MkdirAll(path.Join(rootDir, filepath.Dir(m.Path)), os.ModePerm); err != nil { + output, err := securejoin.SecureJoin(rootDir, m.Path) + if err != nil { + return "", err + } + + if err := os.MkdirAll(filepath.Dir(output), os.ModePerm); err != nil { return "", fmt.Errorf("unable to create dir, error: %w", err) } - filePath := path.Join(rootDir, m.Path) - if err := ioutil.WriteFile(filePath, []byte(m.Content), os.ModePerm); err != nil { + if err := ioutil.WriteFile(output, []byte(m.Content), os.ModePerm); err != nil { return "", fmt.Errorf("unable to write file, error: %w", err) } - - return filePath, nil + return output, nil }