Skip to content

[release-7.4] BulkLoad: reject path-traversal in manifest file paths - #14334

Merged
ploxiln merged 4 commits into
apple:release-7.4from
ShreeChaturvedi:bulkload-path-traversal-7.4
Oct 28, 2026
Merged

ploxiln merged 4 commits into
apple:release-7.4from
ShreeChaturvedi:bulkload-path-traversal-7.4

Conversation

@ShreeChaturvedi

@ShreeChaturvedi ShreeChaturvedi commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Backport of #13665 to release-7.4. release-8.0 already contains it.

A crafted bulk-load manifest could set [RelativePath], [DataFileName], or [ByteSampleFileName] to an absolute path or one containing .., driving resetFileFolder() → eraseDirectoryRecursive() and file writes outside the storage server's local staging root. The fix rejects such fields in BulkLoadFileSet::isValid() and adds a containment check right before the staging folder is reset in both the CP and blobstore transport paths.

The BulkLoading.h change applied cleanly. The fdbserver/core/BulkLoadUtil.cpp hunk conflicted only because release-7.4 has this code in fdbserver/BulkLoadUtil.actor.cpp; the same code was applied there, with the flow/UnitTest.h include placed before actorcompiler.h. No functional differences from main.

Validation:

  • fdbserver builds (clang, USE_WERROR=ON).
  • fdbserver -r unittests -f /fdbserver/BulkLoad/bulkLoadPathIsContained passes.
  • tests/fast/BulkLoading.toml and tests/fast/BulkDumping.toml pass; no SevError, and the new guard never fires on normal jobs.
  • Joshua: 20261020-222819-shreechaturvedi-cb016c5149db79d4: 100,000/100,000 passed, 0 failed.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

…pple#13665)

* BulkLoad: reject path-traversal in manifest file paths

A crafted bulk-load manifest could set [RelativePath], [DataFileName], or
[ByteSampleFileName] to an absolute path or one containing ".." components.
getFolder() joins these onto the local staging root and the result flows into
abspath() -> eraseDirectoryRecursive() (and subsequent file writes), so a
manifest such as "[RelativePath]: ../../../../var/lib/foundationdb" could make
every storage server running the job wipe and overwrite an arbitrary directory
(e.g. the FDB data directory or TLS material).

Add two layers of validation:

- Parse/construction time: BulkLoadFileSet::isValid() now rejects relativePath,
  manifestFileName, dataFileName, and byteSampleFileName that are absolute or
  contain a ".." component (bulkLoadPathIsContained). Bad manifests are refused
  before any local path is constructed.

- Sink guard: assertLocalFolderContained() verifies the resolved staging folder
  is at or under the resolved local root immediately before resetFileFolder(),
  in both bulkLoadTransportCP_impl and bulkLoadTransportBlobstore_impl.

Add unit test /fdbserver/BulkLoad/bulkLoadPathIsContained.

* Formatting

* BulkLoad: use std::string::starts_with in containment check

Address review feedback: C++20 starts_with() is clearer than the
size()+compare() prefix check.

(cherry picked from commit 8f09b3c)
@spraza
spraza force-pushed the bulkload-path-traversal-7.4 branch from 31b1d6b to f5f2ac5 Compare October 26, 2026 17:41
@spraza

spraza commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Change looks isolated to bulk load, so ok for 7.4 backport. Accepting.

@ShreeChaturvedi triggered rebase. Once CI is clear, feel free to merge.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@ploxiln

ploxiln commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator
fdbserver -r simulation --crash --logsize 1024MB -f src/foundationdb/tests/rare/ClogRemoteTLog.toml --buggify on --seed 3177335570

20261028-011744-pr14334-441b0796-17899-7b17973001945297 compressed=True data_size=60629440 duration=294310 ended=10000 fail=1 fail_fast=10 max_runs=10000 pass=9999 priority=100 remaining=0 runtime=0:10:08 sanity=False started=10000 stopped=20261028-012752 submitted=20261028-011744 timeout=5400 username=pr14334-441b0796-17899

unrelated (but noting in case it forms a pattern), will trigger rebuild/retest

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-macos-m1 on macOS 14.x

  • Commit ID: 80580f4
  • Duration 0:35:16
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

@foundationdb-ci

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux RHEL 9

  • Commit ID: 80580f4
  • Duration 0:37:13
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

@ploxiln ploxiln changed the title [release-7.4] BulkLoad: reject path-traversal in manifest file paths (#13665) [release-7.4] BulkLoad: reject path-traversal in manifest file paths Oct 28, 2026
@foundationdb-ci

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang-arm on Linux RHEL 9

  • Commit ID: 80580f4
  • Duration 0:43:39
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

@foundationdb-ci

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux RHEL 9

  • Commit ID: 80580f4
  • Duration 0:51:58
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

@foundationdb-ci

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

  • Commit ID: 80580f4
  • Duration 1:46:12
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)
  • Cluster Test Logs zip file of the test logs (available for 30 days)

@ploxiln
ploxiln merged commit 1fa5be8 into apple:release-7.4 Oct 28, 2026
6 of 7 checks passed
@foundationdb-ci

This comment has been minimized.

@foundationdb-ci

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-macos on macOS 14.x

  • Commit ID: 80580f4
  • Duration 2:55:55
  • Result: ✅ SUCCEEDED
  • Error: N/A
  • Build Log terminal output (available for 30 days)
  • Build Workspace zip file of the working directory (available for 30 days)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants