Default path validation on push (oras-project#66)

Validate name by default

cmd/oras/push.go

@@ -20,10 +20,11 @@ const (
 )
 
 type pushOptions struct {
-	targetRef           string
-	fileRefs            []string
-	manifestConfigRef   string
-	manifestAnnotations string
+	targetRef              string
+	fileRefs               []string
+	manifestConfigRef      string
+	manifestAnnotations    string
+	pathValidationDisabled bool
 
 	debug    bool
 	configs  []string
@@ -60,6 +61,7 @@ Example - Push file "hi.txt" with the custom manifest config "config.json" of th
 
 	cmd.Flags().StringVarP(&opts.manifestConfigRef, "manifest-config", "", "", "manifest config file")
 	cmd.Flags().StringVarP(&opts.manifestAnnotations, "manifest-annotations", "", "", "manifest annotation file")
+	cmd.Flags().BoolVarP(&opts.pathValidationDisabled, "disable-path-validation", "", false, "skip path validation")
 	cmd.Flags().BoolVarP(&opts.debug, "debug", "d", false, "debug mode")
 	cmd.Flags().StringArrayVarP(&opts.configs, "config", "c", nil, "auth config path")
 	cmd.Flags().StringVarP(&opts.username, "username", "u", "", "registry username")
@@ -100,6 +102,9 @@ func runPush(opts pushOptions) error {
 		file.Annotations = nil
 		pushOpts = append(pushOpts, oras.WithConfig(file))
 	}
+	if opts.pathValidationDisabled {
+		pushOpts = append(pushOpts, oras.WithNameValidation(nil))
+	}
 	for _, fileRef := range opts.fileRefs {
 		filename, mediaType := parseFileRef(fileRef, "")
 		name := filepath.Clean(filename)

pkg/content/file.go

@@ -235,7 +235,11 @@ func (s *FileStore) resolveWritePath(name string) (string, error) {
 			return "", err
 		}
 		rel, err := filepath.Rel(base, target)
-		if err != nil || strings.HasPrefix(filepath.ToSlash(rel), "../") {
+		if err != nil {
+			return "", ErrPathTraversalDisallowed
+		}
+		rel = filepath.ToSlash(rel)
+		if strings.HasPrefix(rel, "../") || rel == ".." {
 			return "", ErrPathTraversalDisallowed
 		}
 	}

pkg/oras/errors.go

@@ -7,3 +7,11 @@ var (
 	ErrResolverUndefined = errors.New("resolver_undefined")
 	ErrEmptyDescriptors  = errors.New("empty_descriptors")
 )
+
+// Path validation related errors
+var (
+	ErrDirtyPath               = errors.New("dirty_path")
+	ErrPathNotSlashSeparated   = errors.New("path_not_slash_separated")
+	ErrAbsolutePathDisallowed  = errors.New("absolute_path_disallowed")
+	ErrPathTraversalDisallowed = errors.New("path_traversal_disallowed")
+)

pkg/oras/push.go

@@ -19,19 +19,26 @@ func Push(ctx context.Context, resolver remotes.Resolver, ref string, provider c
 	if len(descriptors) == 0 {
 		return ErrEmptyDescriptors
 	}
-	var opt pushOpts
+	opt := pushOptsDefaults()
 	for _, o := range opts {
-		if err := o(&opt); err != nil {
+		if err := o(opt); err != nil {
 			return err
 		}
 	}
+	if opt.validateName != nil {
+		for _, desc := range descriptors {
+			if err := opt.validateName(desc); err != nil {
+				return err
+			}
+		}
+	}
 
 	pusher, err := resolver.Pusher(ctx, ref)
 	if err != nil {
 		return err
 	}
 
-	desc, provider, err := pack(provider, descriptors, &opt)
+	desc, provider, err := pack(provider, descriptors, opt)
 	if err != nil {
 		return err
 	}

pkg/oras/push_opts.go

@@ -1,11 +1,25 @@
 package oras
 
-import ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+import (
+	"path/filepath"
+	"strings"
+
+	orascontent "github.com/deislabs/oras/pkg/content"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
+)
 
 type pushOpts struct {
 	config              *ocispec.Descriptor
 	configAnnotations   map[string]string
 	manifestAnnotations map[string]string
+	validateName        func(desc ocispec.Descriptor) error
+}
+
+func pushOptsDefaults() *pushOpts {
+	return &pushOpts{
+		validateName: ValidateNameAsPath,
+	}
 }
 
 // PushOpt allows callers to set options on the oras push
@@ -34,3 +48,52 @@ func WithManifestAnnotations(annotations map[string]string) PushOpt {
 		return nil
 	}
 }
+
+// WithNameValidation validates the image title in the descriptor.
+// Pass nil to disable name validation.
+func WithNameValidation(validate func(desc ocispec.Descriptor) error) PushOpt {
+	return func(o *pushOpts) error {
+		o.validateName = validate
+		return nil
+	}
+}
+
+// ValidateNameAsPath validates name in the descriptor as file path in order
+// to generate good packages intended to be pulled using the FileStore or
+// the oras cli.
+// For cross-platform considerations, only unix paths are accepted.
+func ValidateNameAsPath(desc ocispec.Descriptor) error {
+	// no empty name
+	path, ok := orascontent.ResolveName(desc)
+	if !ok || path == "" {
+		return orascontent.ErrNoName
+	}
+
+	// path should be clean
+	if target := filepath.ToSlash(filepath.Clean(path)); target != path {
+		return errors.Wrap(ErrDirtyPath, path)
+	}
+
+	// path should be slash-separated
+	if strings.Contains(path, "\\") {
+		return errors.Wrap(ErrPathNotSlashSeparated, path)
+	}
+
+	// disallow absolute path: covers unix and windows format
+	if strings.HasPrefix(path, "/") {
+		return errors.Wrap(ErrAbsolutePathDisallowed, path)
+	}
+	if len(path) > 2 {
+		c := path[0]
+		if path[1] == ':' && path[2] == '/' && ('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') {
+			return errors.Wrap(ErrAbsolutePathDisallowed, path)
+		}
+	}
+
+	// disallow path traversal
+	if strings.HasPrefix(path, "../") || path == ".." {
+		return errors.Wrap(ErrPathTraversalDisallowed, path)
+	}
+
+	return nil
+}

pkg/oras/push_opts_test.go

@@ -0,0 +1,64 @@
+package oras
+
+import (
+	"testing"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/suite"
+)
+
+type PushOptsSuite struct {
+	suite.Suite
+}
+
+func (suite *PushOptsSuite) TestValidateNameAsPath() {
+	var err error
+
+	// valid path
+	err = ValidateNameAsPath(descFromName("hello.txt"))
+	suite.NoError(err, "valid path")
+	err = ValidateNameAsPath(descFromName("foo/bar"))
+	suite.NoError(err, "valid path with multiple sub-directories")
+
+	// no empty name
+	err = ValidateNameAsPath(descFromName(""))
+	suite.Error(err, "empty path")
+
+	// path should be clean
+	err = ValidateNameAsPath(descFromName("./hello.txt"))
+	suite.Error(err, "dirty path")
+	err = ValidateNameAsPath(descFromName("foo/../bar"))
+	suite.Error(err, "dirty path")
+
+	// path should be slash-separated
+	err = ValidateNameAsPath(descFromName("foo\\bar"))
+	suite.Error(err, "path not slash separated")
+
+	// disallow absolute path
+	err = ValidateNameAsPath(descFromName("/foo/bar"))
+	suite.Error(err, "unix: absolute path disallowed")
+	err = ValidateNameAsPath(descFromName("C:\\foo\\bar"))
+	suite.Error(err, "windows: absolute path disallowed")
+	err = ValidateNameAsPath(descFromName("C:/foo/bar"))
+	suite.Error(err, "windows: absolute path disallowed")
+
+	// disallow path traversal
+	err = ValidateNameAsPath(descFromName(".."))
+	suite.Error(err, "path traversal disallowed")
+	err = ValidateNameAsPath(descFromName("../bar"))
+	suite.Error(err, "path traversal disallowed")
+	err = ValidateNameAsPath(descFromName("foo/../../bar"))
+	suite.Error(err, "path traversal disallowed")
+}
+
+func TestPushOptsSuite(t *testing.T) {
+	suite.Run(t, new(PushOptsSuite))
+}
+
+func descFromName(name string) ocispec.Descriptor {
+	return ocispec.Descriptor{
+		Annotations: map[string]string{
+			ocispec.AnnotationTitle: name,
+		},
+	}
+}