Skip to content

[deckhouse-cli] Reject tar entries escaping the target directory in mirror bundle unpack - #474

Draft
Fkuloff wants to merge 2 commits into
mainfrom
fix/bundle-unpack-path-traversal
Draft

[deckhouse-cli] Reject tar entries escaping the target directory in mirror bundle unpack#474
Fkuloff wants to merge 2 commits into
mainfrom
fix/bundle-unpack-path-traversal

Conversation

@Fkuloff

@Fkuloff Fkuloff commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

bundle.Unpack (used by d8 mirror push) built the on-disk path of every archive entry with filepath.Join(targetPath, "tmp", filepath.Clean(hdr.Name)). Clean keeps a leading .., so an entry named ../../x was written outside the target directory. The same loop dereferenced the tar header before checking a non-EOF error from tar.Reader.Next, which panics on a corrupt archive.

Problem

  • d8 mirror push <bundle> unpacks every .tar it is given before anything is pushed; an entry with .. segments escapes the unpack directory and creates or truncates a file on the operator's host
  • A truncated or non-tar file in the bundle directory crashes d8 with a nil-pointer dereference instead of an error

Fix

  • safeJoin(root, name), private to the bundle package like the guards in the other extractors: joins and rejects a result that leaves root (filepath.Rel check, as in deckhouse-controller); the check is lexical, and Unpack materializes regular files only, so no symlink can redirect it
  • Unpack resolves the write path through it and fails the whole archive on the first escaping entry, before anything is written
  • tar.Reader.Next errors other than io.EOF are returned
  • The staging directory path is built once (tmpDir) instead of three times

Before / After

Real binaries built from main and from this branch, one-entry bundle with ../../../../../ESCAPED/pwn, local registry:

$ d8 mirror push --insecure --file <workdir>/bundle/module-evil.tar localhost:5000/testrepo
# main
ERROR Unpack module-evil failed error="unpack: move module from tmp: read directory: open <workdir>/bundle/.tmp/mirror/push/unified/tmp: no such file or directory"
$ cat <workdir>/bundle/ESCAPED/pwn        # five levels above the unpack root
* * * * * root /bin/sh -c "id > /tmp/pwned"

# this branch
ERROR Unpack module-evil failed error="unpack: path \"../../../../../ESCAPED/pwn\" escapes the target directory"
$ ls <workdir>/bundle/ESCAPED
ls: <workdir>/bundle/ESCAPED: No such file or directory
$ head -c 1024 /dev/urandom > <workdir>/bundle/module-garbage.tar
$ d8 mirror push --insecure --file <workdir>/bundle/module-garbage.tar localhost:5000/testrepo
# main
panic: runtime error: invalid memory address or nil pointer dereference
	.../pkg/libmirror/bundle/bundle.go:69

# this branch
ERROR Unpack module-garbage failed error="unpack: read tar: archive/tar: invalid tar header"

Tests

  • TestSafeJoin — accepted: a/b, dir/foo..bar, a/../b, /abs/x; rejected: .., ../x, a/../../x
  • TestUnpackRejectsPathTraversal — the archive fails and nothing is written inside or outside the unpack directory
  • TestUnpackRejectsCorruptArchive — garbage input returns an error
  • makeIndexTar now delegates its tar-writing tail to the new makeFileTar; existing tests unchanged

Notes

  • The other tar.NewReader users in the CLI compare entry names or read entries into memory and never write an entry path to disk; internal/packagecmd/.../imagefs and internal/cr/.../imagefs keep the guards they already have
  • Absolute entry names were never a vector: filepath.Join places them under the target directory

…irror bundle unpack

Signed-off-by: Artem Kuleshov <artem.kuleshov@flant.com>
@Fkuloff Fkuloff self-assigned this Sep 4, 2026
Signed-off-by: Artem Kuleshov <artem.kuleshov@flant.com>
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.

1 participant