Skip to content

Commit

Permalink
disallow reserved names
Browse files Browse the repository at this point in the history
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
  • Loading branch information
butonic committed Jun 25, 2024
1 parent 9351da5 commit 70165ef
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 19 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/disallow-reserved-names.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion internal/http/services/owncloud/ocdav/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/http/services/owncloud/ocdav/move.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 11 additions & 17 deletions internal/http/services/owncloud/ocdav/tus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -82,19 +76,15 @@ 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 {
w.WriteHeader(http.StatusBadRequest)
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)
}

Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions internal/http/services/owncloud/ocdav/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) == "" {
Expand Down

0 comments on commit 70165ef

Please sign in to comment.