diff --git a/changelog/unreleased/block-overwriting-mountpoints.md b/changelog/unreleased/block-overwriting-mountpoints.md new file mode 100644 index 0000000000..a21fe9c01f --- /dev/null +++ b/changelog/unreleased/block-overwriting-mountpoints.md @@ -0,0 +1,5 @@ +Bugfix: Block overwriting mountpoints + +This blocks overwriting mountpoints through the webdav COPY api. It is now returning a bad request when attempting to overwrite a mountpoint. + +https://github.com/cs3org/reva/pull/4785 diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index c009e59789..22701e007e 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -127,7 +127,7 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string) return } - cp := s.prepareCopy(ctx, w, r, spacelookup.MakeRelativeReference(srcSpace, src, false), spacelookup.MakeRelativeReference(dstSpace, dst, false), &sublog) + cp := s.prepareCopy(ctx, w, r, spacelookup.MakeRelativeReference(srcSpace, src, false), spacelookup.MakeRelativeReference(dstSpace, dst, false), &sublog, dstSpace.GetRoot().GetStorageId() == utils.ShareStorageProviderID) if cp == nil { return } @@ -362,7 +362,7 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s return } - cp := s.prepareCopy(ctx, w, r, &srcRef, &dstRef, &sublog) + cp := s.prepareCopy(ctx, w, r, &srcRef, &dstRef, &sublog, dstRef.GetResourceId().GetStorageId() == utils.ShareStorageProviderID) if cp == nil { return } @@ -552,7 +552,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele return nil } -func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger) *copy { +func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger, destInShareJail bool) *copy { isChild, err := s.referenceIsChildOf(ctx, s.gatewaySelector, dstRef, srcRef) if err != nil { switch err.(type) { @@ -675,11 +675,6 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re if dstStatRes.Status.Code == rpc.Code_CODE_OK { successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.8.5 - if utils.IsSpaceRoot(dstStatRes.GetInfo()) { - log.Error().Msg("overwriting is not allowed") - w.WriteHeader(http.StatusBadRequest) - return nil - } if !overwrite { log.Warn().Bool("overwrite", overwrite).Msg("dst already exists") w.WriteHeader(http.StatusPreconditionFailed) @@ -688,6 +683,38 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re errors.HandleWebdavError(log, w, b, err) // 412, see https://tools.ietf.org/html/rfc4918#section-9.8.5 return nil } + + if utils.IsSpaceRoot(dstStatRes.GetInfo()) { + log.Error().Msg("overwriting is not allowed") + w.WriteHeader(http.StatusBadRequest) + return nil + } + + // we must not allow to override mountpoints - so we check if we have write access to the parent. If not this is a mountpoint + if destInShareJail { + res, err := client.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: dstStatRes.GetInfo().GetParentId()}}) + if err != nil { + log.Error().Err(err).Msg("error sending grpc stat request") + w.WriteHeader(http.StatusInternalServerError) + return nil + } + + switch res.GetStatus().GetCode() { + case rpc.Code_CODE_OK: + // we have write access to the parent - this is not a mountpoint + case rpc.Code_CODE_NOT_FOUND: + // we have no write access to the parent - this is a mountpoint + log.Error().Msg("must not overwrite mount points") + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte("must not overwrite mount points")) + return nil + default: + log.Error().Str("msg", res.GetStatus().GetMessage()).Msg("unexpected status code when checking parent") + w.WriteHeader(http.StatusInternalServerError) + return nil + } + } + // delete existing tree when overwriting a directory or replacing a file with a directory if dstStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER || (dstStatRes.Info.Type == provider.ResourceType_RESOURCE_TYPE_FILE &&