Skip to content

Commit 023ad0e

Browse files
committed
fix: header entries representing root dir error
Some tar utilities, most notably Python's built-in TarFile.add, include a header entry to represent the root of the directory being tarred and also append a slash to header entries representing directories. This means the root directory gets the unfortunate name of "/" which `root.MkdirAll("/")` interprets as an absolute path and errors. Since the root directory already exists at this point, we can just skip the call to create it altogether.
1 parent b6d3e8f commit 023ad0e

File tree

2 files changed

+87
-5
lines changed

2 files changed

+87
-5
lines changed

slug.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,8 @@ func (p *Packer) Unpack(r io.Reader, dst string) error {
429429

430430
// Make the directories to the path using root for security.
431431
dir := filepath.Dir(header.Name)
432-
if dir != "." {
432+
// The directory root already exists so we don't need to create it
433+
if dir != "." && dir != "/" {
433434
// Timestamps and permissions will be restored after all files are extracted.
434435
if err := root.MkdirAll(dir, 0755); err != nil {
435436
return fmt.Errorf("failed to create directory %q: %w", dir, err)

slug_test.go

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,87 @@ func TestUnpack_HeaderOrdering(t *testing.T) {
649649
}
650650
}
651651

652+
func TestUnpack_DirectoryRootHeader(t *testing.T) {
653+
testCases := []string{
654+
".",
655+
"/",
656+
}
657+
for _, tc := range testCases {
658+
t.Run("with directory root '"+tc+"'", func(t *testing.T) {
659+
dir, err := os.MkdirTemp("", "slug")
660+
if err != nil {
661+
t.Fatalf("err:%v", err)
662+
}
663+
defer func() { _ = os.RemoveAll(dir) }()
664+
in := filepath.Join(dir, "slug.tar.gz")
665+
666+
// Create the output file
667+
wfh, err := os.Create(in)
668+
if err != nil {
669+
t.Fatalf("err: %v", err)
670+
}
671+
672+
// Gzip compress all the output data
673+
gzipW := gzip.NewWriter(wfh)
674+
675+
// Tar the file contents
676+
tarW := tar.NewWriter(gzipW)
677+
678+
headers := []struct {
679+
name string
680+
data string
681+
headerType byte
682+
mode int64
683+
}{
684+
{name: tc, data: "", headerType: tar.TypeDir, mode: 0755},
685+
{name: "file.txt", data: "file body", headerType: tar.TypeReg, mode: 0644},
686+
}
687+
688+
for _, header := range headers {
689+
var hdr tar.Header
690+
data := header.data
691+
692+
hdr.Name = header.name
693+
hdr.Typeflag = header.headerType
694+
hdr.Mode = header.mode
695+
if header.headerType == tar.TypeReg {
696+
hdr.Size = int64(len(data))
697+
}
698+
699+
_ = tarW.WriteHeader(&hdr)
700+
if header.headerType == tar.TypeReg {
701+
_, _ = tarW.Write([]byte(data))
702+
}
703+
}
704+
705+
_ = tarW.Close()
706+
_ = gzipW.Close()
707+
_ = wfh.Close()
708+
709+
// Open the slug file for reading.
710+
fh, err := os.Open(in)
711+
if err != nil {
712+
t.Fatalf("err: %v", err)
713+
}
714+
715+
// Create a dir to unpack into.
716+
dst, err := os.MkdirTemp(dir, "")
717+
if err != nil {
718+
t.Fatalf("err: %v", err)
719+
}
720+
defer func() { _ = os.RemoveAll(dst) }()
721+
722+
// Now try unpacking it.
723+
if err := Unpack(fh, dst); err != nil {
724+
t.Fatalf("err: %v", err)
725+
}
726+
727+
// Verify all the files
728+
verifyFile(t, filepath.Join(dst, "file.txt"), 0, "file body")
729+
})
730+
}
731+
}
732+
652733
func TestUnpackDuplicateNoWritePerm(t *testing.T) {
653734
dir, err := os.MkdirTemp("", "slug")
654735
if err != nil {
@@ -1455,7 +1536,7 @@ func TestZipSlipProtection(t *testing.T) {
14551536
if _, err := os.Stat(extractedPath); err != nil {
14561537
t.Fatalf("File should have been safely extracted within destination: %v", err)
14571538
}
1458-
1539+
14591540
t.Log("Zip Slip attack properly contained within extraction directory")
14601541
}
14611542

@@ -1516,7 +1597,7 @@ func TestAbsoluteSymlinkContainment(t *testing.T) {
15161597
if !strings.Contains(err.Error(), "external target") {
15171598
t.Fatalf("Expected 'external target' error for absolute symlink, got: %v", err)
15181599
}
1519-
1600+
15201601
t.Log("Absolute symlink properly rejected")
15211602
}
15221603

@@ -1557,6 +1638,6 @@ func TestPlatformSpecificSecurity(t *testing.T) {
15571638
if _, err := root.Stat(testFile); err != nil {
15581639
t.Fatalf("File should exist within root: %v", err)
15591640
}
1560-
1641+
15611642
t.Logf("Platform %s: os.Root basic operations working", runtime.GOOS)
1562-
}
1643+
}

0 commit comments

Comments
 (0)