diff --git a/common/util/file_helper.go b/common/util/file_helper.go index 090b3c068..46d74102b 100644 --- a/common/util/file_helper.go +++ b/common/util/file_helper.go @@ -1,6 +1,7 @@ package util import ( + "fmt" gopath "path" "path/filepath" "strings" @@ -37,3 +38,12 @@ func AbsolutifyPath(pathToManifest string, pathToFile string, fs boshsys.FileSys return absPath, nil } + +// SafeJoinPath returns filepath.Join(base, untrusted), or an error if +// untrusted is not a local path. +func SafeJoinPath(base, untrusted string) (string, error) { + if !filepath.IsLocal(untrusted) { + return "", fmt.Errorf("path '%s' is not a safe local path", untrusted) + } + return filepath.Join(base, untrusted), nil +} diff --git a/common/util/file_helper_test.go b/common/util/file_helper_test.go index 76baa679d..98e0fb0c7 100644 --- a/common/util/file_helper_test.go +++ b/common/util/file_helper_test.go @@ -11,6 +11,38 @@ import ( "github.com/cloudfoundry/bosh-cli/v7/common/util" ) +var _ = Describe("SafeJoinPath", func() { + It("returns the joined path for a normal relative path", func() { + result, err := util.SafeJoinPath(filepath.Join("/", "base"), filepath.Join("subdir", "file.tgz")) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(filepath.Join("/", "base", "subdir", "file.tgz"))) + }) + + It("rejects a single .. component", func() { + _, err := util.SafeJoinPath(filepath.Join("/", "base"), "../file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("rejects a deeply nested path traversal", func() { + _, err := util.SafeJoinPath(filepath.Join("/", "base"), "../../etc/file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("rejects an absolute path", func() { + _, err := util.SafeJoinPath(filepath.Join("/", "base"), "/etc/file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("rejects an empty path", func() { + _, err := util.SafeJoinPath(filepath.Join("/", "base"), "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) +}) + var _ = Describe("AbsolutifyPath", func() { var realfs boshsys.FileSystem var fakeManifestPath, fakeFilePath string diff --git a/installation/installer.go b/installation/installer.go index efb6aa532..d054ab7cc 100644 --- a/installation/installer.go +++ b/installation/installer.go @@ -7,6 +7,7 @@ import ( bosherr "github.com/cloudfoundry/bosh-utils/errors" boshlog "github.com/cloudfoundry/bosh-utils/logger" + util "github.com/cloudfoundry/bosh-cli/v7/common/util" "github.com/cloudfoundry/bosh-cli/v7/installation/blobextract" biinstallmanifest "github.com/cloudfoundry/bosh-cli/v7/installation/manifest" biui "github.com/cloudfoundry/bosh-cli/v7/ui" @@ -117,8 +118,11 @@ func (i *installer) Cleanup(installation Installation) error { func (i *installer) installPackages(compiledPackages []CompiledPackageRef) error { for _, pkg := range compiledPackages { - err := i.blobExtractor.Extract(pkg.BlobstoreID, pkg.SHA1, filepath.Join(i.target.PackagesPath(), pkg.Name)) + targetDir, err := util.SafeJoinPath(i.target.PackagesPath(), pkg.Name) if err != nil { + return bosherr.Errorf("Invalid package name '%s': must be a safe local path", pkg.Name) + } + if err = i.blobExtractor.Extract(pkg.BlobstoreID, pkg.SHA1, targetDir); err != nil { return bosherr.WrapErrorf(err, "Installing package '%s'", pkg.Name) } } @@ -127,17 +131,17 @@ func (i *installer) installPackages(compiledPackages []CompiledPackageRef) error func (i *installer) installJob(renderedJobRef RenderedJobRef, stage biui.Stage) (installedJob InstalledJob, err error) { err = stage.Perform(fmt.Sprintf("Installing job '%s'", renderedJobRef.Name), func() error { - var stageErr error - jobDir := filepath.Join(i.target.JobsPath(), renderedJobRef.Name) + jobDir, err := util.SafeJoinPath(i.target.JobsPath(), renderedJobRef.Name) + if err != nil { + return bosherr.Errorf("Invalid job name '%s': must be a safe local path", renderedJobRef.Name) + } - stageErr = i.blobExtractor.Extract(renderedJobRef.BlobstoreID, renderedJobRef.SHA1, jobDir) - if stageErr != nil { - return bosherr.WrapErrorf(stageErr, "Extracting blob with ID '%s'", renderedJobRef.BlobstoreID) + if err = i.blobExtractor.Extract(renderedJobRef.BlobstoreID, renderedJobRef.SHA1, jobDir); err != nil { + return bosherr.WrapErrorf(err, "Extracting blob with ID '%s'", renderedJobRef.BlobstoreID) } - stageErr = i.blobExtractor.ChmodExecutables(filepath.Join(jobDir, "bin", "*")) - if stageErr != nil { - return bosherr.WrapErrorf(stageErr, "Chmoding binaries for '%s'", jobDir) + if err = i.blobExtractor.ChmodExecutables(filepath.Join(jobDir, "bin", "*")); err != nil { + return bosherr.WrapErrorf(err, "Chmoding binaries for '%s'", jobDir) } installedJob = NewInstalledJob(renderedJobRef, jobDir) diff --git a/installation/installer_test.go b/installation/installer_test.go index 32a852456..9df024b7e 100644 --- a/installation/installer_test.go +++ b/installation/installer_test.go @@ -140,6 +140,43 @@ var _ = Describe("Installer", func() { }) }) + Describe("Install path traversal protection", func() { + var fakeStage *fakebiui.FakeStage + + BeforeEach(func() { + fakeStage = fakebiui.NewFakeStage() + }) + + It("returns error when a compiled package name contains path traversal", func() { + maliciousRef := CompiledPackageRef{ + Name: "../../path", + Version: "v1", + BlobstoreID: "bid", + SHA1: "sha", + } + releaseJobs := []bireljob.Job{} + mockJobResolver.EXPECT().From(installationManifest).Return(releaseJobs, nil) + mockPackageCompiler.EXPECT().For(releaseJobs, fakeStage).Return([]CompiledPackageRef{maliciousRef}, nil) + + _, err := installer.Install(installationManifest, fakeStage) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("returns error when a rendered job name contains path traversal", func() { + releaseJobs := []bireljob.Job{} + compiledPackages := []CompiledPackageRef{} + jobRef := NewRenderedJobRef("../../path", "fp", "blob-id", "sha") + mockJobResolver.EXPECT().From(installationManifest).Return(releaseJobs, nil) + mockPackageCompiler.EXPECT().For(releaseJobs, fakeStage).Return(compiledPackages, nil) + mockJobRenderer.EXPECT().RenderAndUploadFrom(installationManifest, releaseJobs, fakeStage).Return([]RenderedJobRef{jobRef}, nil) + + _, err := installer.Install(installationManifest, fakeStage) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + }) + Describe("Cleanup", func() { var installation Installation diff --git a/releasedir/fs_blobs_dir.go b/releasedir/fs_blobs_dir.go index 6eda1b658..6d4390280 100644 --- a/releasedir/fs_blobs_dir.go +++ b/releasedir/fs_blobs_dir.go @@ -16,6 +16,7 @@ import ( "github.com/cloudfoundry/bosh-utils/work" "gopkg.in/yaml.v2" + util "github.com/cloudfoundry/bosh-cli/v7/common/util" bicrypto "github.com/cloudfoundry/bosh-cli/v7/crypto" ) @@ -101,6 +102,9 @@ func (d FSBlobsDir) Blobs() ([]Blob, error) { var blobs []Blob for recPath, rec := range schema { + if _, err := util.SafeJoinPath(d.dirPath, recPath); err != nil { + return nil, bosherr.Errorf("Invalid blob path '%s': must be a safe local path", recPath) + } blobs = append(blobs, Blob{ Path: recPath, Size: rec.Size, @@ -193,6 +197,11 @@ func (d FSBlobsDir) removeUnknownBlobs(blobs []Blob) error { } func (d FSBlobsDir) TrackBlob(path string, src io.ReadCloser) (Blob, error) { + safePath, err := util.SafeJoinPath(d.dirPath, path) + if err != nil { + return Blob{}, bosherr.Errorf("Invalid blob path '%s': must be a safe local path", path) + } + tempFile, err := d.fs.TempFile("track-blob") if err != nil { return Blob{}, bosherr.WrapErrorf(err, "Creating temp blob") @@ -239,7 +248,7 @@ func (d FSBlobsDir) TrackBlob(path string, src io.ReadCloser) (Blob, error) { tempFile.Close() //nolint:errcheck - err = d.moveBlobLocally(tempFile.Name(), filepath.Join(d.dirPath, path)) + err = d.moveBlobLocally(tempFile.Name(), safePath) if err != nil { return Blob{}, err } @@ -248,12 +257,17 @@ func (d FSBlobsDir) TrackBlob(path string, src io.ReadCloser) (Blob, error) { } func (d FSBlobsDir) UntrackBlob(path string) error { + safePath, err := util.SafeJoinPath(d.dirPath, path) + if err != nil { + return bosherr.Errorf("Invalid blob path '%s': must be a safe local path", path) + } + blobs, err := d.Blobs() if err != nil { return err } - err = d.fs.RemoveAll(filepath.Join(d.dirPath, path)) + err = d.fs.RemoveAll(safePath) if err != nil { return bosherr.WrapErrorf(err, "Removing blob from blobs/") } diff --git a/releasedir/fs_blobs_dir_test.go b/releasedir/fs_blobs_dir_test.go index 0bc14eedf..82c37db6a 100644 --- a/releasedir/fs_blobs_dir_test.go +++ b/releasedir/fs_blobs_dir_test.go @@ -106,6 +106,34 @@ file2.tgz: Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Unmarshalling blobs index")) }) + + It("returns error for a blob path that escapes the blobs directory via ..", func() { + err := fs.WriteFileString(filepath.Join("/", "dir", "config", "blobs.yml"), ` +../../../../home/file: + size: 100 + object_id: id + sha: sha +`) + Expect(err).ToNot(HaveOccurred()) + + _, err = act() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("returns error for an absolute blob path", func() { + err := fs.WriteFileString(filepath.Join("/", "dir", "config", "blobs.yml"), ` +/etc/file: + size: 100 + object_id: id + sha: sha +`) + Expect(err).ToNot(HaveOccurred()) + + _, err = act() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) }) Describe("SyncBlobs", func() { @@ -600,6 +628,20 @@ file2.tgz: Expect(blobsDir.Blobs()).To(BeEmpty()) }) + + It("returns error for a blob path that escapes the blobs directory via ..", func() { + content := io.NopCloser(strings.NewReader("content")) + _, err := blobsDir.TrackBlob("../../etc/file", content) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("returns error for an absolute blob path", func() { + content := io.NopCloser(strings.NewReader("content")) + _, err := blobsDir.TrackBlob("/etc/file", content) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) }) Describe("UntrackBlob", func() { @@ -714,6 +756,18 @@ bosh-116.tgz: {Path: filepath.Join("dir", "file.tgz"), Size: 133, SHA1: "13e"}, })) }) + + It("returns error for a blob path that escapes the blobs directory via ..", func() { + err := blobsDir.UntrackBlob("../../etc/file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("returns error for an absolute blob path", func() { + err := blobsDir.UntrackBlob("/etc/file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) }) Describe("UploadBlobs", func() { diff --git a/releasedir/index/fs_index_blobs.go b/releasedir/index/fs_index_blobs.go index 7c3a1ab4f..fdfb9ba6e 100644 --- a/releasedir/index/fs_index_blobs.go +++ b/releasedir/index/fs_index_blobs.go @@ -3,7 +3,6 @@ package index import ( "fmt" "os" - "path/filepath" "runtime" "strings" @@ -12,6 +11,8 @@ import ( bosherr "github.com/cloudfoundry/bosh-utils/errors" boshfu "github.com/cloudfoundry/bosh-utils/fileutil" boshsys "github.com/cloudfoundry/bosh-utils/system" + + util "github.com/cloudfoundry/bosh-cli/v7/common/util" ) type FSIndexBlobs struct { @@ -141,5 +142,10 @@ func (c FSIndexBlobs) blobPath(sha1 string) (string, error) { if runtime.GOOS == "windows" { sha1 = strings.ReplaceAll(sha1, ":", "_") } - return filepath.Join(absDirPath, sha1), nil + + candidatePath, err := util.SafeJoinPath(absDirPath, sha1) + if err != nil { + return "", bosherr.Errorf("Invalid blob SHA1 '%s': must be a safe local path", sha1) + } + return candidatePath, nil } diff --git a/releasedir/index/fs_index_blobs_test.go b/releasedir/index/fs_index_blobs_test.go index c72b196c6..13b1fb259 100644 --- a/releasedir/index/fs_index_blobs_test.go +++ b/releasedir/index/fs_index_blobs_test.go @@ -4,7 +4,6 @@ import ( "errors" "os" "path/filepath" - "strings" "syscall" fakeblob "github.com/cloudfoundry/bosh-utils/blobstore/fakes" @@ -115,6 +114,12 @@ var _ = Describe("FSIndexBlobs", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("Cannot find blob named 'name' with SHA1 'sha1'")) }) + + It("returns error if sha1 is a path", func() { + _, err := blobs.Get("name", "blob-id", "../../file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) }) Context("when configured with a blobstore", func() { @@ -160,12 +165,10 @@ var _ = Describe("FSIndexBlobs", func() { Expect(reporter.IndexEntryDownloadFinishedCallCount()).To(Equal(1)) }) - It("returns error if parsing digest string fails", func() { - //currently, the only way to cause a digest parse failure is with an empty string + It("returns error if sha1 is empty", func() { _, err := blobs.Get("name", "blob-id", "") Expect(err).To(HaveOccurred()) - Expect(strings.ToLower(err.Error())).To(ContainSubstring( - "no digest algorithm found. supported algorithms: sha1, sha256, sha512")) + Expect(err.Error()).To(ContainSubstring("safe local path")) }) Context("when downloading blob fails", func() { @@ -284,6 +287,24 @@ var _ = Describe("FSIndexBlobs", func() { Expect(err).ToNot(HaveOccurred()) }) + Context("when sha1 is a path", func() { + BeforeEach(func() { + blobs = boshidx.NewFSIndexBlobs(filepath.Join("/", "dir", "sub-dir"), reporter, nil, fs) + }) + + It("returns error for a path with ..", func() { + _, _, err := blobs.Add("name", filepath.Join("/", "tmp", "payload"), "../../.file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("returns error for an absolute path sha1", func() { + _, _, err := blobs.Add("name", filepath.Join("/", "tmp", "payload"), "/etc/file") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + }) + itCopiesFileIntoDir := func() { It("copies file into cache dir", func() { blobID, path, err := blobs.Add("name", filepath.Join("/", "tmp", "sha1"), "sha1") diff --git a/templatescompiler/job_renderer.go b/templatescompiler/job_renderer.go index 4bee5f1cd..6aebe85df 100644 --- a/templatescompiler/job_renderer.go +++ b/templatescompiler/job_renderer.go @@ -10,6 +10,7 @@ import ( boshsys "github.com/cloudfoundry/bosh-utils/system" boshuuid "github.com/cloudfoundry/bosh-utils/uuid" + util "github.com/cloudfoundry/bosh-cli/v7/common/util" bireljob "github.com/cloudfoundry/bosh-cli/v7/release/job" bierbrenderer "github.com/cloudfoundry/bosh-cli/v7/templatescompiler/erbrenderer" ) @@ -54,11 +55,17 @@ func (r *jobRenderer) Render(releaseJob bireljob.Job, releaseJobProperties *bipr renderedJob := NewRenderedJob(releaseJob, destinationPath, r.fs, r.logger) for src, dst := range releaseJob.Templates { - err := r.renderFile( - filepath.Join(sourcePath, "templates", src), - filepath.Join(destinationPath, dst), - context, - ) + safeSrcPath, err := util.SafeJoinPath(filepath.Join(sourcePath, "templates"), src) + if err != nil { + defer renderedJob.DeleteSilently() + return nil, bosherr.Errorf("Invalid template source '%s': must be a safe local path", src) + } + safeDstPath, err := util.SafeJoinPath(destinationPath, dst) + if err != nil { + defer renderedJob.DeleteSilently() + return nil, bosherr.Errorf("Invalid template destination '%s': must be a safe local path", dst) + } + err = r.renderFile(safeSrcPath, safeDstPath, context) if err != nil { defer renderedJob.DeleteSilently() return nil, bosherr.WrapErrorf(err, "Rendering template src: %s, dst: %s", src, dst) diff --git a/templatescompiler/job_renderer_test.go b/templatescompiler/job_renderer_test.go index acc72d72e..3d87a208a 100644 --- a/templatescompiler/job_renderer_test.go +++ b/templatescompiler/job_renderer_test.go @@ -104,6 +104,24 @@ var _ = Describe("JobRenderer", func() { })) }) + It("returns error if a template dst path is not local", func() { + job.Templates = map[string]string{ + "template.erb": "../../../../home/user/file", + } + _, err := jobRenderer.Render(*job, &releaseJobProperties, jobProperties, globalProperties, "fake-deployment-name", "1.2.3.4") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + + It("returns error if a template src path escapes the templates directory", func() { + job.Templates = map[string]string{ + "../../../../etc/file": "config/out.yml", + } + _, err := jobRenderer.Render(*job, &releaseJobProperties, jobProperties, globalProperties, "fake-deployment-name", "1.2.3.4") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("safe local path")) + }) + Context("when rendering fails", func() { BeforeEach(func() { _ = fakeERBRenderer.SetRenderBehavior( //nolint:errcheck