Skip to content

Fix symlink containment in UbuntuCorpusTrainer.extract() - #2449

Merged
gunthercox merged 5 commits into
masterfrom
fix
Aug 24, 2026
Merged

Fix symlink containment in UbuntuCorpusTrainer.extract()#2449
gunthercox merged 5 commits into
masterfrom
fix

Conversation

@gunthercox

Copy link
Copy Markdown
Owner

No description provided.

gunthercox and others added 4 commits August 22, 2026 16:22
A duplicated assignment of _DIR_FD_EXTRACTION_SUPPORTED overwrote the first
one and dropped the os.path.isdir('/proc/self/fd') term, so the flag became
true on any POSIX platform with O_NOFOLLOW and dir_fd support.

The dir_fd path builds its extraction root as /proc/self/fd/N, so on macOS
and the BSDs that path does not resolve and extract() aborts on the first
write with OSError: [Errno 30] Read-only file system: '/proc'. That makes
UbuntuCorpusTrainer unusable off Linux rather than just unhardened.

Removing the second assignment restores the guard the comment above it
describes. Added a test asserting the flag implies procfs is present.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@arpitjain099

Copy link
Copy Markdown
Contributor

Ran the race against the updated branch. It holds.

Same harness as before, swapping data_directory for a symlink once extraction has visibly started, 30 runs at each size:

members=   50  attacker won 0/30 runs, 0 files written outside
members=  500  attacker won 0/30 runs, 0 files written outside
members= 4000  attacker won 0/30 runs, 0 files written outside

Against 24/30, 27/30 and 27/30 before, with 1168, 13472 and 107971 files landing outside. The swap itself still landed on all 90 runs, so that is the anchoring doing the work rather than the harness missing its window.

I also forced the flag off to exercise the non-dir_fd path, since that is what macOS and the BSDs take: 0/30 at every size there as well. So the staging directory plus the atomic rename closes it on its own, and the fd anchoring is defence in depth rather than the load-bearing part. Worth knowing if you ever need to simplify.

One thing to fix before this merges, which I opened as #2451: _DIR_FD_EXTRACTION_SUPPORTED is assigned twice and the second assignment drops the /proc/self/fd term, so the flag comes out true on macOS where procfs does not exist. The mkdir succeeds because it is genuinely fd-relative, then tarfile writes through /proc/self/fd/N and extract() dies on the first write with OSError: [Errno 30] Read-only file system: '/proc'.

Keep the procfs check in the dir_fd extraction guard
@gunthercox
gunthercox merged commit 47a9d3b into master Aug 24, 2026
8 checks passed
@gunthercox
gunthercox deleted the fix branch August 24, 2026 21:03
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.

2 participants