From 82ca88a2eb24d418c30bf960ef071b0bbec04631 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Wed, 31 Jan 2024 10:41:03 -0500 Subject: [PATCH 1/4] Fix tar path traversal through symlinks (#1) * fix tar path traversal through symlinks Signed-off-by: Alex Goodman * address absolute symlink destinations Signed-off-by: Alex Goodman --------- Signed-off-by: Alex Goodman --- tar.go | 18 +++++++++++ tar_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/tar.go b/tar.go index be898665..d5820114 100644 --- a/tar.go +++ b/tar.go @@ -238,6 +238,24 @@ func (t *Tar) untarNext(destination string) error { return fmt.Errorf("checking path traversal attempt: %v", errPath) } + switch header.Typeflag { + case tar.TypeSymlink, tar.TypeLink: + // this covers cases when the link is an absolute path to a file outside the destination folder + if filepath.IsAbs(header.Linkname) { + errPath := &IllegalPathError{AbsolutePath: "", Filename: header.Linkname} + return fmt.Errorf("absolute path for symlink destination not allowed: %w", errPath) + } + + // though we've already checked the name for possible path traversals, it is possible + // to write content though a symlink to a path outside of the destination folder + // with multiple header entries. We should consider any symlink or hardlink that points + // to outside of the destination folder to be a possible path traversal attack. + errPath = t.CheckPath(destination, header.Linkname) + if errPath != nil { + return fmt.Errorf("checking path traversal attempt in symlink: %w", errPath) + } + } + if t.StripComponents > 0 { if strings.Count(header.Name, "/") < t.StripComponents { return nil // skip path with fewer components diff --git a/tar_test.go b/tar_test.go index 7a9d3541..c365a6d6 100644 --- a/tar_test.go +++ b/tar_test.go @@ -1,16 +1,26 @@ package archiver_test import ( + "archive/tar" + "bytes" "io/ioutil" "os" "path" + "path/filepath" "testing" "github.com/mholt/archiver/v3" ) +func requireDoesNotExist(t *testing.T, path string) { + _, err := os.Lstat(path) + if err == nil { + t.Fatalf("'%s' expected to not exist", path) + } +} + func requireRegularFile(t *testing.T, path string) os.FileInfo { - fileInfo, err := os.Stat(path) + fileInfo, err := os.Lstat(path) if err != nil { t.Fatalf("fileInfo on '%s': %v", path, err) } @@ -47,6 +57,83 @@ func TestDefaultTar_Unarchive_HardlinkSuccess(t *testing.T) { assertSameFile(t, fileaInfo, filebInfo) } +func TestDefaultTar_Unarchive_SymlinkPathTraversal(t *testing.T) { + tmp := t.TempDir() + source := filepath.Join(tmp, "source.tar") + createSymlinkPathTraversalSample(t, source, "./../target") + destination := filepath.Join(tmp, "destination") + + err := archiver.DefaultTar.Unarchive(source, destination) + if err != nil { + t.Fatalf("unarchiving '%s' to '%s': %v", source, destination, err) + } + + requireDoesNotExist(t, filepath.Join(tmp, "target")) + requireRegularFile(t, filepath.Join(tmp, "destination", "duplicatedentry.txt")) +} + +func TestDefaultTar_Unarchive_SymlinkPathTraversal_AbsLinkDestination(t *testing.T) { + tmp := t.TempDir() + source := filepath.Join(tmp, "source.tar") + createSymlinkPathTraversalSample(t, source, "/tmp/thing") + destination := filepath.Join(tmp, "destination") + + err := archiver.DefaultTar.Unarchive(source, destination) + if err != nil { + t.Fatalf("unarchiving '%s' to '%s': %v", source, destination, err) + } + + requireDoesNotExist(t, "/tmp/thing") + requireRegularFile(t, filepath.Join(tmp, "destination", "duplicatedentry.txt")) +} + +func createSymlinkPathTraversalSample(t *testing.T, archivePath string, linkPath string) { + t.Helper() + + type tarinfo struct { + Name string + Link string + Body string + Type byte + } + + var infos = []tarinfo{ + {"duplicatedentry.txt", linkPath, "", tar.TypeSymlink}, + {"duplicatedentry.txt", "", "content modified!", tar.TypeReg}, + } + + var buf bytes.Buffer + var tw = tar.NewWriter(&buf) + for _, ti := range infos { + hdr := &tar.Header{ + Name: ti.Name, + Mode: 0600, + Linkname: ti.Link, + Typeflag: ti.Type, + Size: int64(len(ti.Body)), + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal("Writing header: ", err) + } + if _, err := tw.Write([]byte(ti.Body)); err != nil { + t.Fatal("Writing body: ", err) + } + } + + f, err := os.Create(archivePath) + if err != nil { + t.Fatal(err) + } + _, err = f.Write(buf.Bytes()) + if err != nil { + t.Fatal(err) + } + + if err := f.Close(); err != nil { + t.Fatal(err) + } +} + func TestDefaultTar_Extract_HardlinkSuccess(t *testing.T) { source := "testdata/gnu-hardlinks.tar" From 528262521a951f5f2c95f648e9ed769ffdbc4c7c Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Wed, 31 Jan 2024 16:43:04 -0500 Subject: [PATCH 2/4] use go 1.15 for CI validations Signed-off-by: Alex Goodman --- .github/workflows/macos-latest.yml | 2 +- .github/workflows/ubuntu-latest.yml | 2 +- .github/workflows/windows-latest.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/macos-latest.yml b/.github/workflows/macos-latest.yml index 0b3ffcca..28ff0024 100644 --- a/.github/workflows/macos-latest.yml +++ b/.github/workflows/macos-latest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: - go-version: [1.13, 1.17] + go-version: [1.15] runs-on: macos-latest steps: - name: Install Go diff --git a/.github/workflows/ubuntu-latest.yml b/.github/workflows/ubuntu-latest.yml index d13cd3e0..86132435 100644 --- a/.github/workflows/ubuntu-latest.yml +++ b/.github/workflows/ubuntu-latest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: - go-version: [1.13, 1.17] + go-version: [1.15] runs-on: ubuntu-latest steps: - name: Install Go diff --git a/.github/workflows/windows-latest.yml b/.github/workflows/windows-latest.yml index e9b95d1f..fa94198f 100644 --- a/.github/workflows/windows-latest.yml +++ b/.github/workflows/windows-latest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: - go-version: [1.13, 1.17] + go-version: [1.15] runs-on: windows-latest steps: - name: Install Go From bc17d0260e077ee23b83ea8824b9a48fb68cbb69 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Thu, 1 Feb 2024 11:40:09 -0500 Subject: [PATCH 3/4] use go 1.21 for CI validations Signed-off-by: Alex Goodman --- .github/workflows/macos-latest.yml | 2 +- .github/workflows/ubuntu-latest.yml | 2 +- .github/workflows/windows-latest.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/macos-latest.yml b/.github/workflows/macos-latest.yml index 28ff0024..aa5c5da1 100644 --- a/.github/workflows/macos-latest.yml +++ b/.github/workflows/macos-latest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: - go-version: [1.15] + go-version: [1.21] runs-on: macos-latest steps: - name: Install Go diff --git a/.github/workflows/ubuntu-latest.yml b/.github/workflows/ubuntu-latest.yml index 86132435..e34ac7f2 100644 --- a/.github/workflows/ubuntu-latest.yml +++ b/.github/workflows/ubuntu-latest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: - go-version: [1.15] + go-version: [1.21] runs-on: ubuntu-latest steps: - name: Install Go diff --git a/.github/workflows/windows-latest.yml b/.github/workflows/windows-latest.yml index fa94198f..d10dfc80 100644 --- a/.github/workflows/windows-latest.yml +++ b/.github/workflows/windows-latest.yml @@ -8,7 +8,7 @@ jobs: strategy: matrix: - go-version: [1.15] + go-version: [1.21] runs-on: windows-latest steps: - name: Install Go From e23d3baa92383dfb394f7808ca4544abf7255f0e Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Fri, 26 Apr 2024 14:21:35 -0400 Subject: [PATCH 4/4] use windows absolute path in test Signed-off-by: Alex Goodman --- tar_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tar_test.go b/tar_test.go index c365a6d6..683308ef 100644 --- a/tar_test.go +++ b/tar_test.go @@ -7,6 +7,7 @@ import ( "os" "path" "path/filepath" + "runtime" "testing" "github.com/mholt/archiver/v3" @@ -75,7 +76,13 @@ func TestDefaultTar_Unarchive_SymlinkPathTraversal(t *testing.T) { func TestDefaultTar_Unarchive_SymlinkPathTraversal_AbsLinkDestination(t *testing.T) { tmp := t.TempDir() source := filepath.Join(tmp, "source.tar") - createSymlinkPathTraversalSample(t, source, "/tmp/thing") + + linkPath := "/tmp/thing" + if runtime.GOOS == "windows" { + linkPath = "C:\\tmp\\thing" + } + + createSymlinkPathTraversalSample(t, source, linkPath) destination := filepath.Join(tmp, "destination") err := archiver.DefaultTar.Unarchive(source, destination)