Skip to content

Commit

Permalink
Fix tar path traversal through symlinks (#1)
Browse files Browse the repository at this point in the history
* fix tar path traversal through symlinks

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* address absolute symlink destinations

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
  • Loading branch information
wagoodman committed Jan 31, 2024
1 parent cc194d2 commit 82ca88a
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 1 deletion.
18 changes: 18 additions & 0 deletions tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
89 changes: 88 additions & 1 deletion tar_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
Expand Down Expand Up @@ -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()

Check failure on line 61 in tar_test.go

View workflow job for this annotation

GitHub Actions / build-and-test (1.13)

t.TempDir undefined (type *testing.T has no field or method TempDir)

Check failure on line 61 in tar_test.go

View workflow job for this annotation

GitHub Actions / build-and-test (1.13)

t.TempDir undefined (type *testing.T has no field or method 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()

Check failure on line 76 in tar_test.go

View workflow job for this annotation

GitHub Actions / build-and-test (1.13)

t.TempDir undefined (type *testing.T has no field or method TempDir)

Check failure on line 76 in tar_test.go

View workflow job for this annotation

GitHub Actions / build-and-test (1.13)

t.TempDir undefined (type *testing.T has no field or method 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"

Expand Down

0 comments on commit 82ca88a

Please sign in to comment.