Skip to content

fix(extract): add path traversal guard for tar extraction #92

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skevetter merged 1 commit into from
Apr 24, 2026
Merged

fix(extract): add path traversal guard for tar extraction #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 114 additions & 60 deletions pkg/extract/extract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"strings"
"time"
)
Expand Down Expand Up @@ -67,101 +67,155 @@ func Extract(origReader io.Reader, destFolder string, options ...Option) error {
}
}

func extractNext(tarReader *tar.Reader, destFolder string, options *Options) (bool, error) {
// withinDir checks that resolved stays inside the destFolder boundary.
func withinDir(resolved, destFolder string) bool {
cleanDest := filepath.Clean(destFolder) + string(os.PathSeparator)
return strings.HasPrefix(
filepath.Clean(resolved)+string(os.PathSeparator),
cleanDest,
)
}

// resolveRelativePath strips levels and builds the output path.
func resolveRelativePath(header *tar.Header, opts *Options) string {
rel := getRelativeFromFullPath("/"+header.Name, "")
for i := 0; i < opts.StripLevels; i++ {
rel = strings.TrimPrefix(rel, "/")
idx := strings.Index(rel, "/")
if idx == -1 {
break
}
rel = rel[idx+1:]
}
if opts.StripLevels > 0 {
rel = "/" + rel
}
return rel
}

func extractNext(
tarReader *tar.Reader, destFolder string, options *Options,
) (bool, error) {
header, err := tarReader.Next()
if err != nil {
if !errors.Is(err, io.EOF) {
return false, fmt.Errorf("tar reader next: %w", err)
if errors.Is(err, io.EOF) {
return false, nil
}

return false, nil
return false, fmt.Errorf("tar reader next: %w", err)
}

relativePath := getRelativeFromFullPath("/"+header.Name, "")
if options.StripLevels > 0 {
for i := 0; i < options.StripLevels; i++ {
relativePath = strings.TrimPrefix(relativePath, "/")
index := strings.Index(relativePath, "/")
if index == -1 {
break
}
rel := resolveRelativePath(header, options)
outFileName := filepath.Join(destFolder, rel)

if !withinDir(outFileName, destFolder) {
return false, fmt.Errorf(
"path traversal detected: %s resolves outside destination",
header.Name,
)
}

relativePath = relativePath[index+1:]
switch header.Typeflag {
case tar.TypeSymlink, tar.TypeLink:
if err := validateLinkTarget(header, outFileName, destFolder); err != nil {
return false, err
}
}

relativePath = "/" + relativePath
if err := extractEntry(tarReader, header, outFileName, options); err != nil {
return false, err
}
outFileName := path.Join(destFolder, relativePath)
baseName := path.Dir(outFileName)
return true, nil
}

dirPerm := os.ModePerm
if options.Perm != nil {
dirPerm = *options.Perm
// validateLinkTarget ensures a symlink or hard link target stays within destFolder.
func validateLinkTarget(header *tar.Header, outFileName, destFolder string) error {
linkTarget := resolveLinkTarget(header.Linkname, outFileName)
if !withinDir(linkTarget, destFolder) {
kind := "symlink"
if header.Typeflag == tar.TypeLink {
kind = "hard link"
}
return fmt.Errorf(
"%s traversal detected: %s -> %s",
kind, header.Name, header.Linkname,
)
}
return nil
}

// Check if newer file is there and then don't override?
if err := os.MkdirAll(baseName, dirPerm); err != nil {
return false, err
// resolveLinkTarget resolves a link target to an absolute path.
func resolveLinkTarget(linkname, outFileName string) string {
if filepath.IsAbs(linkname) {
return filepath.Clean(linkname)
}
return filepath.Clean(filepath.Join(filepath.Dir(outFileName), linkname))
}

// whats the file perm?
filePerm := os.FileMode(0o644)
func extractEntry(
tarReader *tar.Reader, header *tar.Header,
outFileName string, options *Options,
) error {
dirPerm := os.ModePerm
if options.Perm != nil {
filePerm = *options.Perm
dirPerm = *options.Perm
}
if err := os.MkdirAll(filepath.Dir(outFileName), dirPerm); err != nil {
return err
}

// Is dir?
switch header.Typeflag {
case tar.TypeDir:
if err := os.MkdirAll(outFileName, dirPerm); err != nil {
return false, err
}

return true, nil
return os.MkdirAll(outFileName, dirPerm)
case tar.TypeSymlink:
err := os.Symlink(header.Linkname, outFileName)
if err != nil {
return false, err
}

return true, nil
return os.Symlink(header.Linkname, outFileName)
case tar.TypeLink:
err := os.Link(header.Linkname, outFileName)
if err != nil {
return false, err
}

return true, nil
return os.Link(header.Linkname, outFileName)
default:
return extractRegularFile(tarReader, header, outFileName, options)
}
Comment on lines +146 to 175

@coderabbitai coderabbitai Bot Apr 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In Go's os.Link(oldname, newname), how are relative oldname paths resolved?

💡 Result:

In Go, os.Link(oldname, newname) passes oldname and newname strings directly to the platform’s hard-link syscall (syscall.Link on Unix/POSIX), without any Go-level “base directory” handling for relative paths. Therefore: - If oldname is a relative path, it is resolved by the operating system relative to the process’s current working directory at the time of the call. - newname is likewise resolved relative to the process’s current working directory. Evidence: - The os.Link implementation on Unix calls syscall.Link(oldname, newname) with the original strings (no filepath.Abs/Join/Clean-to-CWD step is performed).

Citations:

🏁 Script executed:

# First, let's look at the full context of the extract.go file around the mentioned lines
cd /tmp && find . -name "extract.go" -type f 2>/dev/null | head -5

Repository: devsy-org/devsy

Length of output: 41

🏁 Script executed:

# Search for the file in the repository
git ls-files | grep -i extract.go

Repository: devsy-org/devsy

Length of output: 82

🏁 Script executed:

# Look for hard link validation logic
rg "tar\.TypeLink|os\.Link" --type go -B 3 -A 3

Repository: devsy-org/devsy

Length of output: 1544

🏁 Script executed:

# Check if destFolder is defined and available in the extractEntry context
rg "destFolder|destdir" --type go -B 2 -A 2 | head -50

Repository: devsy-org/devsy

Length of output: 2500

🏁 Script executed:

# Get the full extract.go file to understand complete flow
wc -l pkg/extract/extract.go

Repository: devsy-org/devsy

Length of output: 86

🏁 Script executed:

# Read the validateLinkTarget and surrounding functions
sed -n '100,250p' pkg/extract/extract.go

Repository: devsy-org/devsy

Length of output: 3405

🏁 Script executed:

# Also check the complete extractNext and extractEntry functions
sed -n '50,200p' pkg/extract/extract.go

Repository: devsy-org/devsy

Length of output: 3882

🌐 Web query:

tar format hard link symlink Linkname relative path archive root semantics

💡 Result:

In the tar archive format (ustar/POSIX), the linkname field (bytes 157-256 in the 512-byte header) stores the target path for both hard links (typeflag '1') and symbolic links (typeflag '2'). The linkname is a null-terminated string (unless full 100 bytes used). It records the target's pathname as it was on the original filesystem at archive creation time. This is typically a relative path from the directory containing the link, without using the prefix field (prefix is only for the name field of the entry itself). When extracting: - For symlinks ('2'): The symlink is created at the extracted path (name/prefix), pointing exactly to the linkname string as stored. If linkname has '..' components, the symlink will traverse up the extraction root. GNU tar delays such symlinks until end to avoid cycles but creates them relative to the extraction root. - For hard links ('1'): The link is created to the previously extracted file whose archived name matches the linkname. linkname must refer to an earlier entry in the archive. If it has '..' (relative), GNU tar resolves it against the archive's path structure; absolute or '..' may be delayed or fail if target not found. Hard links do not dereference; they reference the archive entry. Path semantics relative to archive root: tar paths (name and linkname) are treated relative to the extraction directory. GNU tar strips leading '/' from paths by default (use --absolute-names to preserve). linkname paths are not automatically relativized or absolutized; they are literal strings interpreted relative to the current extraction root. This allows links to span outside the root if '..' or absolute paths are used, but tools like GNU tar may restrict dangerous traversals for security. PAX extensions allow longer paths via extended headers, but ustar linkname remains 100 bytes limited.

Citations:

Hard link resolution does not match os.Link semantics, creating a security/correctness mismatch.

validateLinkTarget resolves relative Linkname against the parent directory of the link being created, but extractEntry passes the raw header.Linkname to os.Link, which resolves relative paths against the process CWD — not the extraction root. This inconsistency causes two problems:

  1. Security issue: The traversal check validates a different path than the one actually linked. An archive with a relative hard link like ../../../etc/passwd would be validated against the archive extraction structure but at extraction time could resolve to a completely different inode if the CWD differs from the extraction directory.

  2. Correctness issue: Legitimate relative hard links in archives (which should reference previously extracted entries in the archive) will fail or point to the wrong file when the process CWD is not the extraction directory.

The fix is to resolve hard link targets consistently with the validation logic. Unlike symlinks — which store the target literally and resolve at access time relative to the symlink's directory — hard links must resolve to the correct inode at creation time.

🔧 Suggested direction 
 case tar.TypeLink:
-    return os.Link(header.Linkname, outFileName)
+    // Linkname in tar is relative to the extraction root; resolve it consistently
+    // with validation to ensure the linked inode is correct.
+    target := header.Linkname
+    if !filepath.IsAbs(target) {
+        target = filepath.Join(destFolder, target)
+    }
+    return os.Link(target, outFileName)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/extract/extract.go` around lines 146 - 175, extractEntry is calling
os.Link with header.Linkname, which resolves relative paths against the process
CWD and mismatches the validation logic; instead resolve hardlink targets the
same way as validation does. Update the tar.TypeLink branch in extractEntry to
call resolveLinkTarget(header.Linkname, outFileName) (or otherwise join/clean
relative link targets against filepath.Dir(outFileName)) and pass that resolved
path into os.Link; keep validateLinkTarget's semantics in mind so validation and
creation both use the same resolution. Ensure you reference resolveLinkTarget
and extractEntry when making the change.

}

// Create / Override file
outFile, err := os.OpenFile(outFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, filePerm)
func extractRegularFile(
tarReader *tar.Reader,
header *tar.Header,
outFileName string,
options *Options,
) error {
filePerm := os.FileMode(0o644)
if options.Perm != nil {
filePerm = *options.Perm
}
outFile, err := openFileWithRetry(outFileName, filePerm)
if err != nil {
// Try again after 5 seconds
time.Sleep(time.Second * 5)
outFile, err = os.OpenFile(outFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, filePerm)
if err != nil {
return false, fmt.Errorf("create %s: %w", outFileName, err)
}
return err
}
defer func() { _ = outFile.Close() }()

if _, err := io.Copy(outFile, tarReader); err != nil {
return false, fmt.Errorf("io copy tar reader %s: %w", outFileName, err)
return fmt.Errorf("io copy tar reader %s: %w", outFileName, err)
}
if err := outFile.Close(); err != nil {
return false, fmt.Errorf("out file close %s: %w", outFileName, err)
return fmt.Errorf("out file close %s: %w", outFileName, err)
}

// Set permissions
if options.Perm == nil {
_ = os.Chmod(outFileName, header.FileInfo().Mode()|0o600) // #nosec G703
_ = os.Chmod(outFileName, header.FileInfo().Mode()|0o600)
}

// Set mod time from tar header
_ = os.Chtimes(outFileName, time.Now(), header.FileInfo().ModTime())
return nil
}

return true, nil
func openFileWithRetry(name string, perm os.FileMode) (*os.File, error) {
flags := os.O_RDWR | os.O_CREATE | os.O_TRUNC
f, err := os.OpenFile(filepath.Clean(name), flags, perm)
if err != nil {
time.Sleep(time.Second * 5)
f, err = os.OpenFile(filepath.Clean(name), flags, perm)
if err != nil {
return nil, fmt.Errorf("create %s: %w", name, err)
}
}
return f, nil
}

func getRelativeFromFullPath(fullpath string, prefix string) string {
Expand Down
Loading
Loading