diff --git a/changelog/unreleased/disallow-reserved-names.md b/changelog/unreleased/disallow-reserved-names.md new file mode 100644 index 00000000000..9cb927f732e --- /dev/null +++ b/changelog/unreleased/disallow-reserved-names.md @@ -0,0 +1,5 @@ +Bugfix: Disallow reserved filenames + +We now disallow the reserved `..` and `.` filenames. They are only allowed as destinations of move or copy operations. + +https://github.com/cs3org/reva/pull/4740 diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 010b2615c9e..5d9b0252d22 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -95,7 +95,7 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string) return } - if err := ValidateName(path.Base(dst), s.nameValidators); err != nil { + if err := ValidateDestination(path.Base(dst), s.nameValidators); err != nil { w.WriteHeader(http.StatusBadRequest) b, err := errors.Marshal(http.StatusBadRequest, "destination failed naming rules", "") errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err) diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index ca203a19a4f..8d31b4a9944 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -67,7 +67,7 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string) return } - if err := ValidateName(path.Base(dstPath), s.nameValidators); err != nil { + if err := ValidateDestination(path.Base(dstPath), s.nameValidators); err != nil { w.WriteHeader(http.StatusBadRequest) b, err := errors.Marshal(http.StatusBadRequest, "destination naming rules", "") errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index c22651978a7..c235b14fcb8 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -58,21 +58,15 @@ func (s *svc) handlePathTusPost(w http.ResponseWriter, r *http.Request, ns strin // read filename from metadata meta := tusd.ParseMetadataHeader(r.Header.Get(net.HeaderUploadMetadata)) - if err := ValidateName(path.Base(meta["filename"]), s.nameValidators); err != nil { - w.WriteHeader(http.StatusPreconditionFailed) - return - } // append filename to current dir - fn := path.Join(ns, r.URL.Path, meta["filename"]) - - sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() - // check tus headers? - ref := &provider.Reference{ - // FIXME ResourceId? - Path: fn, + // a path based request has no resource id, so we can only provide a path. The gateway has te figure out which provider is responsible + Path: path.Join(ns, r.URL.Path, meta["filename"]), } + + sublog := appctx.GetLogger(ctx).With().Str("path", r.URL.Path).Str("filename", meta["filename"]).Logger() + s.handleTusPost(ctx, w, r, meta, ref, sublog) } @@ -82,12 +76,6 @@ func (s *svc) handleSpacesTusPost(w http.ResponseWriter, r *http.Request, spaceI // read filename from metadata meta := tusd.ParseMetadataHeader(r.Header.Get(net.HeaderUploadMetadata)) - if err := ValidateName(path.Base(meta["filename"]), s.nameValidators); err != nil { - w.WriteHeader(http.StatusPreconditionFailed) - return - } - - sublog := appctx.GetLogger(ctx).With().Str("spaceid", spaceID).Str("path", r.URL.Path).Logger() ref, err := spacelookup.MakeStorageSpaceReference(spaceID, path.Join(r.URL.Path, meta["filename"])) if err != nil { @@ -95,6 +83,8 @@ func (s *svc) handleSpacesTusPost(w http.ResponseWriter, r *http.Request, spaceI return } + sublog := appctx.GetLogger(ctx).With().Str("spaceid", spaceID).Str("path", r.URL.Path).Str("filename", meta["filename"]).Logger() + s.handleTusPost(ctx, w, r, meta, &ref, sublog) } @@ -116,6 +106,10 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. w.WriteHeader(http.StatusPreconditionFailed) return } + if err := ValidateName(path.Base(meta["filename"]), s.nameValidators); err != nil { + w.WriteHeader(http.StatusPreconditionFailed) + return + } // Test if the target is a secret filedrop var isSecretFileDrop bool diff --git a/internal/http/services/owncloud/ocdav/validation.go b/internal/http/services/owncloud/ocdav/validation.go index 168b3106b59..174bf126253 100644 --- a/internal/http/services/owncloud/ocdav/validation.go +++ b/internal/http/services/owncloud/ocdav/validation.go @@ -27,6 +27,9 @@ func ValidatorsFromConfig(c *config.Config) []Validator { // ValidateName will validate a file or folder name, returning an error when it is not accepted func ValidateName(name string, validators []Validator) error { + if err := notReserved()(name); err != nil { + return fmt.Errorf("name validation failed: %w", err) + } for _, v := range validators { if err := v(name); err != nil { return fmt.Errorf("name validation failed: %w", err) @@ -35,6 +38,25 @@ func ValidateName(name string, validators []Validator) error { return nil } +// ValidateDestination will validate a file or folder destination name (which can be . or ..), returning an error when it is not accepted +func ValidateDestination(name string, validators []Validator) error { + for _, v := range validators { + if err := v(name); err != nil { + return fmt.Errorf("name validation failed: %w", err) + } + } + return nil +} + +func notReserved() Validator { + return func(s string) error { + if s == ".." || s == "." { + return errors.New(". and .. are reserved names") + } + return nil + } +} + func notEmpty() Validator { return func(s string) error { if strings.TrimSpace(s) == "" {