Skip to content

Commit

Permalink
fix(ocdav): block overwriting mountpoints
Browse files Browse the repository at this point in the history
Signed-off-by: jkoberg <jkoberg@owncloud.com>
  • Loading branch information
kobergj committed Jul 31, 2024
1 parent 548644c commit eb9b157
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 8 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/block-overwriting-mountpoints.md
Original file line number Diff line number Diff line change
@@ -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
43 changes: 35 additions & 8 deletions internal/http/services/owncloud/ocdav/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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 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 access to the parent - this is not a mountpoint
case rpc.Code_CODE_NOT_FOUND:
// we have no 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 &&
Expand Down

0 comments on commit eb9b157

Please sign in to comment.