Skip to content

Commit f2f2dff

Browse files
committed
fix(lib): add path traversal protection to tarball extraction
- Add sanitizeTarPath() function with three-layer validation: - Reject absolute paths - Reject paths starting with ".." after cleaning - Verify resolved path stays within destination directory - Skip symlinks and hard links in extractTarball() for security - Add comprehensive test coverage in lib/fetch_test.go: - Version parsing validation (6 subtests) - Checksum verification robustness (5 subtests) - Path traversal attack prevention (5 subtests)
1 parent ae3755b commit f2f2dff

2 files changed

Lines changed: 602 additions & 1 deletion

File tree

lib/fetch.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,12 @@ func extractTarball(tarball, destDir string) error {
337337

338338
tr := tar.NewReader(gzr)
339339

340+
// Resolve destDir to absolute path for security checks
341+
absDestDir, err := filepath.Abs(destDir)
342+
if err != nil {
343+
return fmt.Errorf("resolving destination directory: %w", err)
344+
}
345+
340346
for {
341347
header, err := tr.Next()
342348
if err == io.EOF {
@@ -346,7 +352,11 @@ func extractTarball(tarball, destDir string) error {
346352
return err
347353
}
348354

349-
target := filepath.Join(destDir, header.Name)
355+
// Security: Validate path to prevent path traversal attacks
356+
target, err := sanitizeTarPath(absDestDir, header.Name)
357+
if err != nil {
358+
return err
359+
}
350360

351361
switch header.Typeflag {
352362
case tar.TypeDir:
@@ -368,8 +378,40 @@ func extractTarball(tarball, destDir string) error {
368378
return err
369379
}
370380
outFile.Close()
381+
case tar.TypeSymlink, tar.TypeLink:
382+
// Skip symlinks and hard links for security - they could point outside destDir
383+
continue
371384
}
372385
}
373386

374387
return nil
375388
}
389+
390+
// sanitizeTarPath validates that a tar entry path is safe to extract.
391+
// It prevents path traversal attacks by ensuring the resolved path
392+
// stays within the destination directory.
393+
func sanitizeTarPath(destDir, entryName string) (string, error) {
394+
// Reject absolute paths
395+
if filepath.IsAbs(entryName) {
396+
return "", fmt.Errorf("path traversal detected: absolute path %q not allowed", entryName)
397+
}
398+
399+
// Clean the path to resolve . and .. components
400+
cleanName := filepath.Clean(entryName)
401+
402+
// Reject paths that start with .. after cleaning
403+
if strings.HasPrefix(cleanName, "..") {
404+
return "", fmt.Errorf("path traversal detected: %q escapes destination directory", entryName)
405+
}
406+
407+
// Construct the full target path
408+
target := filepath.Join(destDir, cleanName)
409+
410+
// Final check: ensure the resolved path is within destDir
411+
// This catches edge cases where filepath.Join might not prevent traversal
412+
if !strings.HasPrefix(target, destDir+string(filepath.Separator)) && target != destDir {
413+
return "", fmt.Errorf("path traversal detected: %q resolves outside destination directory", entryName)
414+
}
415+
416+
return target, nil
417+
}

0 commit comments

Comments
 (0)