Repository navigation
Confine catar extraction to the target root (symlink-safe) - #336
Merged
Merged
Conversation
LocalFS computed every destination as filepath.Join(fs.Root, n.Name) and wrote via raw os.OpenFile/RemoveAll/Symlink/Chown/Chmod/Mknod with no symlink protection. An archive could plant a symlink (e.g. evil -> /etc) and then add a later entry with a clean name (evil/passwd) whose write, remove, chown or chmod would be resolved by the kernel through the planted symlink, escaping the extraction root. This worked even without ".." in names, so lexical filtering alone was insufficient. Route all LocalFS write/metadata operations through a lazily-opened, cached os.Root handle anchored to the extraction root. os.Root refuses to traverse any path component or symlink that escapes the root while still following benign in-root relative symlinks, neutralizing the symlink- follow class regardless of the entry name. Symlink targets are kept verbatim (legitimate, matches GNU tar/casync); safety comes from never following an escaping link, not from rejecting the target. os.Root has no Mknod, so CreateDevice creates the node with unix.Mknodat against a confined parent-directory fd. xattrs are applied via fd-based xattr.FSet on a root-opened handle (no symlink follow). golang.org/x/sys is promoted to a direct dependency. As defense-in-depth, ArchiveDecoder now rejects FormatFilename names that are empty, ".", "..", absolute, or contain a path separator, and asserts each cumulative entry path stays within the root. This also stops TarWriter from forwarding poisoned names into produced tar archives. Add adversarial tests covering the reported exploit, lexical escape, verbatim absolute symlink targets, decoder rejection, and a regression guard for benign in-root relative symlinks.
unix.Mknodat does not exist on Darwin (macOS has no mknodat(2) syscall), breaking the macOS CI build. Move the node-creation primitive into a per-platform createDeviceNode: - linux: unix.Mknodat against a confined parent-directory fd obtained through os.Root (strong, race-free). - other Unix (darwin/bsd): validate the parent resolves within the root via os.Root, then path-based unix.Mknod on the resolved real path. Extraction is single-threaded, so no concurrent attacker can swap a component after the check. The rest of CreateDevice (Remove/Chown/Chmod/Chtimes/xattr) stays in localfs_other.go and continues to go through os.Root on all non-Windows platforms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
LocalFScomputed every destination asfilepath.Join(fs.Root, n.Name)and wrote via rawos.OpenFile/os.RemoveAll/os.Symlink/os.Chown/syscall.Chmod/syscall.Mknodwith no symlink protection (noO_NOFOLLOW, noopenat2/RESOLVE_BENEATH, no confinement check).An attacker-controlled catar could:
evil -> /etc(target written verbatim).evil/passwd— a clean name with no.., so lexical filtering does not catch it. The kernel resolvesevilthrough the planted symlink and the write/remove/chown/chmod lands on/etc/passwd, outside the extraction root.Running as root, this also lets the archive set arbitrary ownership/mode on the escaped target.
Fix
LocalFSrouted throughos.Root(Go 1.25): a lazily-opened, cached root handle anchors all write/metadata operations to the extraction root.os.Rootrefuses to traverse any path component or symlink that escapes the root, while still following benign in-root relative symlinks — neutralizing the symlink-follow class regardless of the entry name.CreateDeviceusesunix.Mknodatagainst a confined parent-directory fd (os.Roothas noMknod); xattrs applied via fd-basedxattr.FSet.golang.org/x/syspromoted to a direct dependency.ArchiveDecoderrejectsFormatFilenamenames that are empty,.,.., absolute, or contain a path separator, and asserts each cumulative entry path stays within root. This also stopsTarWriterfrom forwarding poisoned names into produced tar archives.cmd/desync untarcloses the writer handle after extraction.Tests
New adversarial tests: reported symlink-write escape (out-of-root file left untouched), lexical
..escape, verbatim absolute symlink target with following refused, decoder rejection of embedded-slash names, and a regression guard for benign in-root relative symlinks.go test ./...,go vet ./..., andGOOS=windows go build ./cmd/desyncall pass with no regressions.