Skip to content

Commit

Permalink
fix: enforce path validation for push/attach and improve path travers…
Browse files Browse the repository at this point in the history
…al failure message for pull (oras-project#988)

Signed-off-by: suganyas <ssuganyatce@gmail.com>
Co-authored-by: Billy Zha <qweeah@gmail.com>
Co-authored-by: Shiwei Zhang <shizh@microsoft.com>
  • Loading branch information
3 people committed Aug 3, 2023
1 parent 51ac344 commit 59f2682
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 2 deletions.
22 changes: 22 additions & 0 deletions cmd/oras/internal/option/packer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ import (
"errors"
"fmt"
"os"
"path/filepath"
"strings"

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
Expand All @@ -38,6 +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")
)

// Packer option struct.
Expand Down Expand Up @@ -69,6 +72,25 @@ func (opts *Packer) ExportManifest(ctx context.Context, fetcher content.Fetcher,
}
return os.WriteFile(opts.ManifestExportPath, manifestBytes, 0666)
}
func (opts *Packer) Parse() error {
if !opts.PathValidationDisabled {
var failedPaths []string
for _, path := range opts.FileRefs {
// Remove the type if specified in the path <file>[:<type>] format
path, _, err := fileref.Parse(path, "")
if err != nil {
return err
}
if filepath.IsAbs(path) {
failedPaths = append(failedPaths, path)
}
}
if len(failedPaths) > 0 {
return fmt.Errorf("%w: %v", errPathValidation, strings.Join(failedPaths, ", "))
}
}
return nil
}

// LoadManifestAnnotations loads the manifest annotation map.
func (opts *Packer) LoadManifestAnnotations() (annotations map[string]map[string]string, err error) {
Expand Down
1 change: 0 additions & 1 deletion cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions cmd/oras/root/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package root

import (
"context"
"errors"
"fmt"
"io"
"sync"
Expand Down Expand Up @@ -237,6 +238,9 @@ 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 errors.Is(err, file.ErrPathTraversalDisallowed) {
err = fmt.Errorf("%s: %w", "use flag --allow-path-traversal to allow insecurely pulling files outside of working directory", err)
}
return err
}
if pulledEmpty {
Expand Down
1 change: 0 additions & 1 deletion cmd/oras/root/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 59f2682

Please sign in to comment.