From 92201043def6aba9f021f0289180c59424adb057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 25 Jun 2024 11:10:50 +0200 Subject: [PATCH] disallow reserved names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/disallow-reserved-names.md | 5 ++++ internal/http/services/owncloud/ocdav/copy.go | 2 +- internal/http/services/owncloud/ocdav/move.go | 2 +- internal/http/services/owncloud/ocdav/tus.go | 28 ++++++++----------- .../services/owncloud/ocdav/validation.go | 14 ++++++++++ 5 files changed, 32 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/disallow-reserved-names.md diff --git a/changelog/unreleased/disallow-reserved-names.md b/changelog/unreleased/disallow-reserved-names.md new file mode 100644 index 0000000000..9cb927f732 --- /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 010b2615c9..5d9b0252d2 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 ca203a19a4..8d31b4a994 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 c22651978a..c235b14fcb 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 168b3106b5..07b576feeb 100644 --- a/internal/http/services/owncloud/ocdav/validation.go +++ b/internal/http/services/owncloud/ocdav/validation.go @@ -27,6 +27,11 @@ 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 { + return ValidateDestination(name, append(validators, notReserved())) +} + +// 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) @@ -35,6 +40,15 @@ func ValidateName(name string, validators []Validator) error { 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) == "" {