From c1cc13bb3960ca104c31e1c2b92c792f24e3ac5d Mon Sep 17 00:00:00 2001 From: npu Date: Fri, 15 May 2026 22:41:19 +0300 Subject: [PATCH] Security: enforce postprocessing quarantine on the storage download path The processing/antivirus quarantine was only enforced by the ocdav GET/HEAD/PROPFIND handlers via a separate Stat call. Decomposedfs.Download and DownloadRevision did not check the node's processing state, so the datagateway /data endpoint and revision downloads could serve unscanned/in-process content and the ocdav path was racy (TOCTOU). - reject Download/DownloadRevision with errtypes.TooEarly while the node IsProcessing, enforcing the quarantine at the storage layer for every caller - map the too-early error to HTTP 425 in the data transfer handler --- .../enforce-postprocessing-quarantine-on-download.md | 11 +++++++++++ pkg/rhttp/datatx/utils/download/download.go | 3 +++ pkg/storage/utils/decomposedfs/decomposedfs.go | 9 +++++++++ pkg/storage/utils/decomposedfs/revisions.go | 8 ++++++++ 4 files changed, 31 insertions(+) create mode 100644 changelog/unreleased/enforce-postprocessing-quarantine-on-download.md diff --git a/changelog/unreleased/enforce-postprocessing-quarantine-on-download.md b/changelog/unreleased/enforce-postprocessing-quarantine-on-download.md new file mode 100644 index 00000000000..7da3b4edde7 --- /dev/null +++ b/changelog/unreleased/enforce-postprocessing-quarantine-on-download.md @@ -0,0 +1,11 @@ +Security: Enforce postprocessing quarantine on the storage download path + +The "still being processed" quarantine (used e.g. for antivirus scanning) was +only enforced by the ocdav GET/HEAD/PROPFIND handlers via a separate Stat +call. `Decomposedfs.Download` and `Decomposedfs.DownloadRevision` did not +check the node's processing state, so the datagateway `/data` endpoint and +revision downloads could serve unscanned/in-process content, and the ocdav +path was racy. The processing state is now enforced at the storage layer and +the data transfer handler returns `425 Too Early` accordingly. + +https://github.com/owncloud/reva/pull/596 diff --git a/pkg/rhttp/datatx/utils/download/download.go b/pkg/rhttp/datatx/utils/download/download.go index d4b5fbd31e9..47b3382798f 100644 --- a/pkg/rhttp/datatx/utils/download/download.go +++ b/pkg/rhttp/datatx/utils/download/download.go @@ -275,6 +275,9 @@ func handleError(w http.ResponseWriter, log *zerolog.Logger, err error, action s case errtypes.Aborted: log.Debug().Err(err).Str("action", action).Msg("etags do not match") w.WriteHeader(http.StatusPreconditionFailed) + case errtypes.IsTooEarly: + log.Debug().Err(err).Str("action", action).Msg("resource still being processed") + w.WriteHeader(http.StatusTooEarly) default: log.Error().Err(err).Str("action", action).Msg("unexpected error") w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index c4c4fd1e08f..05d519d99f4 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -1133,6 +1133,15 @@ func (fs *Decomposedfs) Download(ctx context.Context, ref *provider.Reference, o return nil, nil, errtypes.NotFound(f) } + // Do not serve the blob while the node is still being post-processed + // (e.g. virus scan in progress). This must be enforced here, at the + // storage layer, so every caller (ocdav, the datagateway /data endpoint, + // the archiver, ...) is covered and the check cannot be raced against a + // preceding Stat. + if n.IsProcessing(ctx) { + return nil, nil, errtypes.TooEarly(n.ID) + } + ri, err := n.AsResourceInfo(ctx, rp, nil, []string{"size", "mimetype", "etag"}, true) if err != nil { return nil, nil, err diff --git a/pkg/storage/utils/decomposedfs/revisions.go b/pkg/storage/utils/decomposedfs/revisions.go index 0948529f5bf..c4fc075e4f6 100644 --- a/pkg/storage/utils/decomposedfs/revisions.go +++ b/pkg/storage/utils/decomposedfs/revisions.go @@ -152,6 +152,14 @@ func (fs *Decomposedfs) DownloadRevision(ctx context.Context, ref *provider.Refe return nil, nil, errtypes.NotFound(f) } + // Do not serve a revision blob while the node is still being + // post-processed (e.g. virus scan in progress). Enforced at the storage + // layer so the revision download path cannot be used to bypass the + // quarantine. + if n.IsProcessing(ctx) { + return nil, nil, errtypes.TooEarly(n.ID) + } + contentPath := fs.lu.InternalPath(spaceID, revisionKey) blobid, blobsize, err := fs.lu.ReadBlobIDAndSizeAttr(ctx, contentPath, nil)