diff --git a/changelog/unreleased/tus-illegal-names.md b/changelog/unreleased/tus-illegal-names.md new file mode 100644 index 00000000000..95b501e5fff --- /dev/null +++ b/changelog/unreleased/tus-illegal-names.md @@ -0,0 +1,5 @@ +Enhancement: Check for illegal names while uploading or moving files + +The code was not checking for invalid file names during uploads and moves. + +https://github.com/cs3org/reva/pull/1900 diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 49131375744..e93b5c41445 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -44,6 +44,14 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string) w.WriteHeader(http.StatusBadRequest) return } + + for _, r := range nameRules { + if !r.Test(dstPath) { + w.WriteHeader(http.StatusBadRequest) + return + } + } + dstPath = path.Join(ns, dstPath) sublog := appctx.GetLogger(ctx).With().Str("src", srcPath).Str("dst", dstPath).Logger() diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 93134dd96dc..d13cb6000a6 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -54,8 +54,31 @@ const ( var ( errInvalidValue = errors.New("invalid value") + + nameRules = [...]nameRule{ + nameNotEmpty{}, + nameDoesNotContain{chars: "\f\r\n\\"}, + } ) +type nameRule interface { + Test(name string) bool +} + +type nameNotEmpty struct{} + +func (r nameNotEmpty) Test(name string) bool { + return len(strings.TrimSpace(name)) > 0 +} + +type nameDoesNotContain struct { + chars string +} + +func (r nameDoesNotContain) Test(name string) bool { + return !strings.ContainsAny(name, r.chars) +} + func init() { global.Register("ocdav", New) } diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 91c07e7becd..0c7f7ccbb28 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -44,9 +44,11 @@ func (s *svc) handlePathTusPost(w http.ResponseWriter, r *http.Request, ns strin // read filename from metadata meta := tusd.ParseMetadataHeader(r.Header.Get(HeaderUploadMetadata)) - if meta["filename"] == "" { - w.WriteHeader(http.StatusPreconditionFailed) - return + for _, r := range nameRules { + if !r.Test(meta["filename"]) { + w.WriteHeader(http.StatusPreconditionFailed) + return + } } // append filename to current dir diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index d83949df4b9..c386485d294 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -58,14 +58,8 @@ Basic file management like up and download, move, copy, properties, quota, trash - [apiWebdavUpload2/uploadFileUsingOldChunking.feature:43](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUpload2/uploadFileUsingOldChunking.feature#L43) #### [invalid file-names should not be created using the TUS protocol](https://github.com/owncloud/ocis/issues/1001) -- [apiWebdavUploadTUS/uploadFile.feature:143](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L143) -- [apiWebdavUploadTUS/uploadFile.feature:144](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L144) - [apiWebdavUploadTUS/uploadFile.feature:145](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L145) -- [apiWebdavUploadTUS/uploadFile.feature:146](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L146) -- [apiWebdavUploadTUS/uploadFile.feature:147](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L147) -- [apiWebdavUploadTUS/uploadFile.feature:148](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L148) - [apiWebdavUploadTUS/uploadFile.feature:149](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L149) -- [apiWebdavUploadTUS/uploadFile.feature:150](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L150) #### [upload a file using TUS resource URL as an other user should not work](https://github.com/owncloud/ocis/issues/1141) - [apiWebdavUploadTUS/uploadFile.feature:165](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L165) diff --git a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md index 58db83e2df8..e9ccbc1a610 100644 --- a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md +++ b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md @@ -89,14 +89,8 @@ The following scenarios fail on OWNCLOUD storage but not on OCIS storage: - [apiWebdavUpload1/uploadFile.feature:113](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUpload1/uploadFile.feature#L113) #### [invalid file-names should not be created using the TUS protocol](https://github.com/owncloud/ocis/issues/1001) -- [apiWebdavUploadTUS/uploadFile.feature:143](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L135) -- [apiWebdavUploadTUS/uploadFile.feature:144](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L136) - [apiWebdavUploadTUS/uploadFile.feature:145](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L137) -- [apiWebdavUploadTUS/uploadFile.feature:146](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L138) -- [apiWebdavUploadTUS/uploadFile.feature:147](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L139) -- [apiWebdavUploadTUS/uploadFile.feature:148](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L140) - [apiWebdavUploadTUS/uploadFile.feature:149](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L141) -- [apiWebdavUploadTUS/uploadFile.feature:150](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L142) ### [500 Internal Server Error on Post request for TUS upload](https://github.com/owncloud/ocis/issues/1047) diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md index b04dea5dead..f2525db2e47 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -63,14 +63,8 @@ Basic file management like up and download, move, copy, properties, quota, trash - [apiWebdavUpload2/uploadFileUsingOldChunking.feature:43](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUpload2/uploadFileUsingOldChunking.feature#L43) #### [invalid file-names should not be created using the TUS protocol](https://github.com/owncloud/ocis/issues/1001) -- [apiWebdavUploadTUS/uploadFile.feature:143](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L135) -- [apiWebdavUploadTUS/uploadFile.feature:144](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L136) - [apiWebdavUploadTUS/uploadFile.feature:145](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L137) -- [apiWebdavUploadTUS/uploadFile.feature:146](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L138) -- [apiWebdavUploadTUS/uploadFile.feature:147](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L139) -- [apiWebdavUploadTUS/uploadFile.feature:148](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L140) - [apiWebdavUploadTUS/uploadFile.feature:149](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L141) -- [apiWebdavUploadTUS/uploadFile.feature:150](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L142) #### [upload a file using TUS resource URL as an other user should not work](https://github.com/owncloud/ocis/issues/1141) - [apiWebdavUploadTUS/uploadFile.feature:165](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavUploadTUS/uploadFile.feature#L155)