From 1ebb3285205202eb7d59957223367b1efc037db1 Mon Sep 17 00:00:00 2001 From: suganyas Date: Mon, 26 Jun 2023 22:14:27 +1000 Subject: [PATCH 01/28] Add changes for fixing the absolute path behaviour in oras push and attach Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 31 ++++++++++++++++++++++++++++++ cmd/oras/root/attach.go | 1 - cmd/oras/root/pull.go | 6 ++++++ cmd/oras/root/push.go | 1 - 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 7fe058b83..52acecef3 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -38,6 +39,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") + errPathValidation = errors.New("one or more files are not in the current directory.If it's intentional use --disable-path-validation flag to skip this check") ) // Packer option struct. @@ -69,6 +71,35 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } +func (opts *Packer) Parse() error { + currentDir, err := os.Getwd() + var failedPaths []string + if err != nil { + return err + } + if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { + for _, path := range opts.FileRefs { + //Remove the type if specified in the path [:] format + lastIndex := strings.LastIndex(path, ":") + if lastIndex != -1 { + path = path[:lastIndex] + } + absPath, err := filepath.Abs(path) + dirPath := filepath.Dir(absPath) + if err != nil { + return err + } + if dirPath != currentDir { + failedPaths = append(failedPaths, absPath) + } + } + if len(failedPaths) > 0 { + errorMsg := fmt.Sprintf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) + return errors.New(errorMsg) + } + } + return nil +} // LoadManifestAnnotations loads the manifest annotation map. func (opts *Packer) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) { diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index a1032f67b..796427bf8 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -113,7 +113,6 @@ func runAttach(ctx context.Context, opts attachOptions) error { return err } defer store.Close() - store.AllowPathTraversalOnWrite = opts.PathValidationDisabled dst, err := opts.NewTarget(opts.Common) if err != nil { diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index ec25da1c0..1f229d941 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -17,8 +17,10 @@ package root import ( "context" + "errors" "fmt" "io" + "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -237,6 +239,10 @@ func runPull(ctx context.Context, opts pullOptions) error { // Copy desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { + if strings.Contains(err.Error(), "path traversal disallowed") { + errorMsg := fmt.Sprintf("%v: %v ", err, "To enable path traversal use --allow-path-traversal flag") + return errors.New(errorMsg) + } return err } if pulledEmpty { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 152a2b2e1..698d3e57d 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -139,7 +139,6 @@ func runPush(ctx context.Context, opts pushOptions) error { return err } defer store.Close() - store.AllowPathTraversalOnWrite = opts.PathValidationDisabled if opts.manifestConfigRef != "" { path, cfgMediaType, err := fileref.Parse(opts.manifestConfigRef, oras.MediaTypeUnknownConfig) if err != nil { From a68ed83f146cd975c2ac749452c1dc99e254a3ec Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:05:12 +1000 Subject: [PATCH 02/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 52acecef3..23c3ce4e5 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -72,8 +72,8 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } func (opts *Packer) Parse() error { - currentDir, err := os.Getwd() var failedPaths []string + currentDir, err := os.Getwd() if err != nil { return err } From 3337a5eeec944d33ddc485b0a192c3181def4bc0 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:24:18 +1000 Subject: [PATCH 03/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 23c3ce4e5..e8bae5827 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -94,8 +94,7 @@ func (opts *Packer) Parse() error { } } if len(failedPaths) > 0 { - errorMsg := fmt.Sprintf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) - return errors.New(errorMsg) + return fmt.Errorf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) } } return nil From f62a8b43237a6d7513b70617298f226c1ce5775d Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:25:01 +1000 Subject: [PATCH 04/28] Update cmd/oras/root/pull.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 1f229d941..9be7e1f8a 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -240,7 +240,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if strings.Contains(err.Error(), "path traversal disallowed") { - errorMsg := fmt.Sprintf("%v: %v ", err, "To enable path traversal use --allow-path-traversal flag") + errorMsg := fmt.Sprintf("%v: %w ", "to enable path traversal use --allow-path-traversal flag", err) return errors.New(errorMsg) } return err From 10901c0bd1213edbf5d766e14e34134ad7667552 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 20:25:36 +1000 Subject: [PATCH 05/28] Address review comments from Billy and tested code Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 12 ++++-------- cmd/oras/root/pull.go | 6 ++---- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index e8bae5827..de8c29636 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -27,6 +27,7 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/pflag" "oras.land/oras-go/v2/content" + "oras.land/oras/cmd/oras/internal/fileref" ) // Pre-defined annotation keys for annotation file @@ -80,17 +81,12 @@ func (opts *Packer) Parse() error { if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format - lastIndex := strings.LastIndex(path, ":") - if lastIndex != -1 { - path = path[:lastIndex] - } - absPath, err := filepath.Abs(path) - dirPath := filepath.Dir(absPath) + path, _, err = fileref.Parse(path, "") if err != nil { return err } - if dirPath != currentDir { - failedPaths = append(failedPaths, absPath) + if filepath.IsAbs(path) { + failedPaths = append(failedPaths, path) } } if len(failedPaths) > 0 { diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 9be7e1f8a..f68188035 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "io" - "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -239,9 +238,8 @@ func runPull(ctx context.Context, opts pullOptions) error { // Copy desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { - if strings.Contains(err.Error(), "path traversal disallowed") { - errorMsg := fmt.Sprintf("%v: %w ", "to enable path traversal use --allow-path-traversal flag", err) - return errors.New(errorMsg) + if errors.Is(err, file.ErrPathTraversalDisallowed) { + err = fmt.Errorf("%s: %w", "use option -T/allow-path-traversal to allow pulling outside of working directory", err) } return err } From 08c45936a487e372972e16dc83f7229bd5f54e0d Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:20 +1000 Subject: [PATCH 06/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index de8c29636..f80fbe3bc 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -40,7 +40,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") - errPathValidation = errors.New("one or more files are not in the current directory.If it's intentional use --disable-path-validation flag to skip this check") + errPathValidation = errors.New("absolute file path detected. If it's intentional use --disable-path-validation flag to skip this check") ) // Packer option struct. From 43929395bc4cbd1b6296bee416402749f0dec944 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:36 +1000 Subject: [PATCH 07/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index f80fbe3bc..b3e34081e 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -74,10 +74,6 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } func (opts *Packer) Parse() error { var failedPaths []string - currentDir, err := os.Getwd() - if err != nil { - return err - } if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format From a8d7abf0d690f683d82b9479d8a938972567503a Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:49 +1000 Subject: [PATCH 08/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index b3e34081e..dfee018e3 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -86,7 +86,7 @@ func (opts *Packer) Parse() error { } } if len(failedPaths) > 0 { - return fmt.Errorf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) + return fmt.Errorf("%w: %v", errPathValidation, strings.Join(failedPaths, ", ")) } } return nil From 74ef5572649fd41e3833935595f4db17269117bc Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:58 +1000 Subject: [PATCH 09/28] Update cmd/oras/root/pull.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index f68188035..d4d27b9a0 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -239,7 +239,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { - err = fmt.Errorf("%s: %w", "use option -T/allow-path-traversal to allow pulling outside of working directory", err) + err = fmt.Errorf("%s: %w", "use flag -T/allow-path-traversal to allow pulling files outside of working directory", err) } return err } From e4179549916e8c7e7f46eebe7ba5cddbb26ec563 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:51:09 +1000 Subject: [PATCH 10/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index dfee018e3..0114fe449 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -74,7 +74,7 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } func (opts *Packer) Parse() error { var failedPaths []string - if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { + if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format path, _, err = fileref.Parse(path, "") From ea6600c7c8026be26e1d197981a58fa1a906acd2 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:55:14 +1000 Subject: [PATCH 11/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 0114fe449..aeda50b73 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -76,7 +76,7 @@ func (opts *Packer) Parse() error { var failedPaths []string if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { - //Remove the type if specified in the path [:] format + // Remove the type if specified in the path [:] format path, _, err = fileref.Parse(path, "") if err != nil { return err From e0d769f0a2347ff52289e9cb0fa374fbb058f22c Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:57:41 +1000 Subject: [PATCH 12/28] Fix the build for compilation error Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index aeda50b73..2a1dfdcea 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -77,7 +77,7 @@ func (opts *Packer) Parse() error { if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { // Remove the type if specified in the path [:] format - path, _, err = fileref.Parse(path, "") + path, _, err := fileref.Parse(path, "") if err != nil { return err } From a548ad4cb264dc713439e138e2429196090a5eff Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Tue, 27 Jun 2023 13:53:44 +0000 Subject: [PATCH 13/28] test(e2e): add test for pushing via absolute path Signed-off-by: Billy Zha --- test/e2e/suite/command/attach.go | 28 ++++++++++++++++++++++++++ test/e2e/suite/command/push.go | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/test/e2e/suite/command/attach.go b/test/e2e/suite/command/attach.go index 54c35f6c3..41632e0f0 100644 --- a/test/e2e/suite/command/attach.go +++ b/test/e2e/suite/command/attach.go @@ -104,6 +104,7 @@ var _ = Describe("Common registry users:", func() { fetched := ORAS("manifest", "fetch", RegistryRef(Host, testRepo, index.Manifests[0].Digest.String())).Exec().Out.Contents() MatchFile(filepath.Join(tempDir, exportName), string(fetched), DefaultTimeout) }) + It("should attach a file via a OCI Image", func() { testRepo := attachTestRepo("image") tempDir := PrepareTempFiles() @@ -121,6 +122,33 @@ var _ = Describe("Common registry users:", func() { Expect(len(index.Manifests)).To(Equal(1)) Expect(index.Manifests[0].MediaType).To(Equal(ocispec.MediaTypeImageManifest)) }) + + It("should attach file with path validation disabled", func() { + testRepo := attachTestRepo("simple") + absAttachFileName := filepath.Join(PrepareTempFiles(), foobar.AttachFileName) + + subjectRef := RegistryRef(Host, testRepo, foobar.Tag) + prepare(RegistryRef(Host, ImageRepo, foobar.Tag), subjectRef) + statusKey := foobar.AttachFileStateKey + statusKey.Name = absAttachFileName + ORAS("attach", "--artifact-type", "test.attach", subjectRef, fmt.Sprintf("%s:%s", absAttachFileName, foobar.AttachFileMedia), "--disable-path-validation"). + MatchStatus([]match.StateKey{statusKey}, false, 1). + Exec() + }) + + It("should fail path validation when attaching file with absolute path", func() { + testRepo := attachTestRepo("simple") + absAttachFileName := filepath.Join(PrepareTempFiles(), foobar.AttachFileName) + + subjectRef := RegistryRef(Host, testRepo, foobar.Tag) + prepare(RegistryRef(Host, ImageRepo, foobar.Tag), subjectRef) + statusKey := foobar.AttachFileStateKey + statusKey.Name = absAttachFileName + ORAS("attach", "--artifact-type", "test.attach", subjectRef, fmt.Sprintf("%s:%s", absAttachFileName, foobar.AttachFileMedia)). + ExpectFailure(). + Exec() + }) + It("should attach a file via a OCI Artifact", func() { testRepo := attachTestRepo("artifact") tempDir := PrepareTempFiles() diff --git a/test/e2e/suite/command/push.go b/test/e2e/suite/command/push.go index dfa56e283..17139fed4 100644 --- a/test/e2e/suite/command/push.go +++ b/test/e2e/suite/command/push.go @@ -25,6 +25,7 @@ import ( "github.com/onsi/gomega" . "github.com/onsi/gomega" "github.com/onsi/gomega/gbytes" + "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras/test/e2e/internal/testdata/feature" "oras.land/oras/test/e2e/internal/testdata/foobar" @@ -65,6 +66,39 @@ var _ = Describe("Remote registry users:", func() { Expect(manifest.Layers).Should(ContainElements(foobar.BlobBarDescriptor("application/vnd.oci.image.layer.v1.tar"))) }) + It("should push files with path validation disabled", func() { + repo := fmt.Sprintf("%s/%s", repoPrefix, "disable-path-validation") + ref := RegistryRef(Host, repo, tag) + absBarName := filepath.Join(PrepareTempFiles(), foobar.FileBarName) + + ORAS("push", ref, absBarName, "-v", "--disable-path-validation"). + Exec() + + // validate + fetched := ORAS("manifest", "fetch", ref).Exec().Out.Contents() + var manifest ocispec.Manifest + Expect(json.Unmarshal(fetched, &manifest)).ShouldNot(HaveOccurred()) + Expect(manifest.Layers).Should(ContainElements(ocispec.Descriptor{ + MediaType: "application/vnd.oci.image.layer.v1.tar", + Digest: digest.Digest(foobar.BarBlobDigest), + Size: 3, + Annotations: map[string]string{ + "org.opencontainers.image.title": absBarName, + }, + })) + }) + + It("should fail path validation when pushing file with absolute path", func() { + repo := fmt.Sprintf("%s/%s", repoPrefix, "path-validation") + ref := RegistryRef(Host, repo, tag) + absBarName := filepath.Join(PrepareTempFiles(), foobar.FileBarName) + // test + ORAS("push", ref, absBarName, "-v"). + MatchErrKeyWords("--disable-path-validation"). + ExpectFailure(). + Exec() + }) + It("should push files and tag", func() { repo := fmt.Sprintf("%s/%s", repoPrefix, "multi-tag") tempDir := PrepareTempFiles() From 755a83c30df7ee3dcaf43ab372afd336618170fb Mon Sep 17 00:00:00 2001 From: suganyas Date: Mon, 26 Jun 2023 22:14:27 +1000 Subject: [PATCH 14/28] Add changes for fixing the absolute path behaviour in oras push and attach Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 31 ++++++++++++++++++++++++++++++ cmd/oras/root/attach.go | 1 - cmd/oras/root/pull.go | 6 ++++++ cmd/oras/root/push.go | 1 - 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 7fe058b83..52acecef3 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -38,6 +39,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") + errPathValidation = errors.New("one or more files are not in the current directory.If it's intentional use --disable-path-validation flag to skip this check") ) // Packer option struct. @@ -69,6 +71,35 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } +func (opts *Packer) Parse() error { + currentDir, err := os.Getwd() + var failedPaths []string + if err != nil { + return err + } + if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { + for _, path := range opts.FileRefs { + //Remove the type if specified in the path [:] format + lastIndex := strings.LastIndex(path, ":") + if lastIndex != -1 { + path = path[:lastIndex] + } + absPath, err := filepath.Abs(path) + dirPath := filepath.Dir(absPath) + if err != nil { + return err + } + if dirPath != currentDir { + failedPaths = append(failedPaths, absPath) + } + } + if len(failedPaths) > 0 { + errorMsg := fmt.Sprintf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) + return errors.New(errorMsg) + } + } + return nil +} // LoadManifestAnnotations loads the manifest annotation map. func (opts *Packer) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) { diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index a1032f67b..796427bf8 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -113,7 +113,6 @@ func runAttach(ctx context.Context, opts attachOptions) error { return err } defer store.Close() - store.AllowPathTraversalOnWrite = opts.PathValidationDisabled dst, err := opts.NewTarget(opts.Common) if err != nil { diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index ec25da1c0..1f229d941 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -17,8 +17,10 @@ package root import ( "context" + "errors" "fmt" "io" + "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -237,6 +239,10 @@ func runPull(ctx context.Context, opts pullOptions) error { // Copy desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { + if strings.Contains(err.Error(), "path traversal disallowed") { + errorMsg := fmt.Sprintf("%v: %v ", err, "To enable path traversal use --allow-path-traversal flag") + return errors.New(errorMsg) + } return err } if pulledEmpty { diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 152a2b2e1..698d3e57d 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -139,7 +139,6 @@ func runPush(ctx context.Context, opts pushOptions) error { return err } defer store.Close() - store.AllowPathTraversalOnWrite = opts.PathValidationDisabled if opts.manifestConfigRef != "" { path, cfgMediaType, err := fileref.Parse(opts.manifestConfigRef, oras.MediaTypeUnknownConfig) if err != nil { From caec5ff1f5693a4172ac60a2cabbfd6781115001 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:05:12 +1000 Subject: [PATCH 15/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 52acecef3..23c3ce4e5 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -72,8 +72,8 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } func (opts *Packer) Parse() error { - currentDir, err := os.Getwd() var failedPaths []string + currentDir, err := os.Getwd() if err != nil { return err } From 007b2d95ef4136f616ea5e18285c70060d6fa861 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:24:18 +1000 Subject: [PATCH 16/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 23c3ce4e5..e8bae5827 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -94,8 +94,7 @@ func (opts *Packer) Parse() error { } } if len(failedPaths) > 0 { - errorMsg := fmt.Sprintf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) - return errors.New(errorMsg) + return fmt.Errorf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) } } return nil From 8f447bd43a8df08241846dde4bf9ebde0371cfc0 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 17:25:01 +1000 Subject: [PATCH 17/28] Update cmd/oras/root/pull.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 1f229d941..9be7e1f8a 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -240,7 +240,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if strings.Contains(err.Error(), "path traversal disallowed") { - errorMsg := fmt.Sprintf("%v: %v ", err, "To enable path traversal use --allow-path-traversal flag") + errorMsg := fmt.Sprintf("%v: %w ", "to enable path traversal use --allow-path-traversal flag", err) return errors.New(errorMsg) } return err From de4bdb7c2e2d94aee8da35bc7be990011287f283 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 20:25:36 +1000 Subject: [PATCH 18/28] Address review comments from Billy and tested code Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 12 ++++-------- cmd/oras/root/pull.go | 6 ++---- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index e8bae5827..de8c29636 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -27,6 +27,7 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/pflag" "oras.land/oras-go/v2/content" + "oras.land/oras/cmd/oras/internal/fileref" ) // Pre-defined annotation keys for annotation file @@ -80,17 +81,12 @@ func (opts *Packer) Parse() error { if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format - lastIndex := strings.LastIndex(path, ":") - if lastIndex != -1 { - path = path[:lastIndex] - } - absPath, err := filepath.Abs(path) - dirPath := filepath.Dir(absPath) + path, _, err = fileref.Parse(path, "") if err != nil { return err } - if dirPath != currentDir { - failedPaths = append(failedPaths, absPath) + if filepath.IsAbs(path) { + failedPaths = append(failedPaths, path) } } if len(failedPaths) > 0 { diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 9be7e1f8a..f68188035 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "io" - "strings" "sync" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -239,9 +238,8 @@ func runPull(ctx context.Context, opts pullOptions) error { // Copy desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { - if strings.Contains(err.Error(), "path traversal disallowed") { - errorMsg := fmt.Sprintf("%v: %w ", "to enable path traversal use --allow-path-traversal flag", err) - return errors.New(errorMsg) + if errors.Is(err, file.ErrPathTraversalDisallowed) { + err = fmt.Errorf("%s: %w", "use option -T/allow-path-traversal to allow pulling outside of working directory", err) } return err } From 351a1100705409d550bf516252afa6703d52eaeb Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:20 +1000 Subject: [PATCH 19/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index de8c29636..f80fbe3bc 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -40,7 +40,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") - errPathValidation = errors.New("one or more files are not in the current directory.If it's intentional use --disable-path-validation flag to skip this check") + errPathValidation = errors.New("absolute file path detected. If it's intentional use --disable-path-validation flag to skip this check") ) // Packer option struct. From e03dec73d264c4a3260b29f49da6983ad0eb691b Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:36 +1000 Subject: [PATCH 20/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index f80fbe3bc..b3e34081e 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -74,10 +74,6 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } func (opts *Packer) Parse() error { var failedPaths []string - currentDir, err := os.Getwd() - if err != nil { - return err - } if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format From 8f03f50a08aae5c5e1618a9e0e60a984959b2140 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:49 +1000 Subject: [PATCH 21/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index b3e34081e..dfee018e3 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -86,7 +86,7 @@ func (opts *Packer) Parse() error { } } if len(failedPaths) > 0 { - return fmt.Errorf("%v: %v currentDir :%v", errPathValidation, strings.Join(failedPaths, ", "), currentDir) + return fmt.Errorf("%w: %v", errPathValidation, strings.Join(failedPaths, ", ")) } } return nil From af7c0ea10c818fc82c1e8e9d8fc1e8d8c6e52835 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:50:58 +1000 Subject: [PATCH 22/28] Update cmd/oras/root/pull.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index f68188035..d4d27b9a0 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -239,7 +239,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { - err = fmt.Errorf("%s: %w", "use option -T/allow-path-traversal to allow pulling outside of working directory", err) + err = fmt.Errorf("%s: %w", "use flag -T/allow-path-traversal to allow pulling files outside of working directory", err) } return err } From 2609e2f505c2a1772cbf4f7a65a36fcc1c05a2b8 Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:51:09 +1000 Subject: [PATCH 23/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index dfee018e3..0114fe449 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -74,7 +74,7 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, } func (opts *Packer) Parse() error { var failedPaths []string - if !opts.PathValidationDisabled && len(opts.FileRefs) != 0 { + if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { //Remove the type if specified in the path [:] format path, _, err = fileref.Parse(path, "") From d1dd5261e57e732f47ddd7ee050023b972a42d8c Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:55:14 +1000 Subject: [PATCH 24/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Billy Zha Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 0114fe449..aeda50b73 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -76,7 +76,7 @@ func (opts *Packer) Parse() error { var failedPaths []string if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { - //Remove the type if specified in the path [:] format + // Remove the type if specified in the path [:] format path, _, err = fileref.Parse(path, "") if err != nil { return err From 0c656eeb2fc3ed628e8a9d398bed1932d26d45ce Mon Sep 17 00:00:00 2001 From: suganyas Date: Tue, 27 Jun 2023 22:57:41 +1000 Subject: [PATCH 25/28] Fix the build for compilation error Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index aeda50b73..2a1dfdcea 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -77,7 +77,7 @@ func (opts *Packer) Parse() error { if !opts.PathValidationDisabled { for _, path := range opts.FileRefs { // Remove the type if specified in the path [:] format - path, _, err = fileref.Parse(path, "") + path, _, err := fileref.Parse(path, "") if err != nil { return err } From 8420368d774b405b7b04c5ec4279899b7d48e9ad Mon Sep 17 00:00:00 2001 From: suganyas Date: Wed, 28 Jun 2023 19:50:10 +1000 Subject: [PATCH 26/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Shiwei Zhang Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 2a1dfdcea..63f7beb10 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -40,7 +40,7 @@ var ( errAnnotationConflict = errors.New("`--annotation` and `--annotation-file` cannot be both specified") errAnnotationFormat = errors.New("missing key in `--annotation` flag") errAnnotationDuplication = errors.New("duplicate annotation key") - errPathValidation = errors.New("absolute file path detected. If it's intentional use --disable-path-validation flag to skip this check") + errPathValidation = errors.New("absolute file path detected. If it's intentional, use --disable-path-validation flag to skip this check") ) // Packer option struct. From f3549cd7dcaba1d7b89b2fe497cc0fc302efeee9 Mon Sep 17 00:00:00 2001 From: suganyas Date: Wed, 28 Jun 2023 19:50:18 +1000 Subject: [PATCH 27/28] Update cmd/oras/root/pull.go Co-authored-by: Shiwei Zhang Signed-off-by: suganyas --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index d4d27b9a0..703833d90 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -239,7 +239,7 @@ func runPull(ctx context.Context, opts pullOptions) error { desc, err := oras.Copy(ctx, src, opts.Reference, dst, opts.Reference, copyOptions) if err != nil { if errors.Is(err, file.ErrPathTraversalDisallowed) { - err = fmt.Errorf("%s: %w", "use flag -T/allow-path-traversal to allow pulling files outside of working directory", err) + err = fmt.Errorf("%s: %w", "use flag --allow-path-traversal to allow insecurely pulling files outside of working directory", err) } return err } From ae964ecc291f2e702bd1fef544e69f2ec28f6d0c Mon Sep 17 00:00:00 2001 From: suganyas Date: Wed, 28 Jun 2023 19:50:31 +1000 Subject: [PATCH 28/28] Update cmd/oras/internal/option/packer.go Co-authored-by: Shiwei Zhang Signed-off-by: suganyas --- cmd/oras/internal/option/packer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/internal/option/packer.go b/cmd/oras/internal/option/packer.go index 63f7beb10..d32fa62de 100644 --- a/cmd/oras/internal/option/packer.go +++ b/cmd/oras/internal/option/packer.go @@ -73,8 +73,8 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher, return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666) } func (opts *Packer) Parse() error { - var failedPaths []string if !opts.PathValidationDisabled { + var failedPaths []string for _, path := range opts.FileRefs { // Remove the type if specified in the path [:] format path, _, err := fileref.Parse(path, "")