Skip to content
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.

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

Fix tar path traversal through symlinks #1

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
12 changes: 12 additions & 0 deletions tar.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,18 @@ func (t *Tar) untarNext(destination string) error {
return fmt.Errorf("checking path traversal attempt: %v", errPath)
}

switch header.Typeflag {
case tar.TypeSymlink, tar.TypeLink:
// 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: %v", errPath)
}
}

if t.StripComponents > 0 {
if strings.Count(header.Name, "/") < t.StripComponents {
return nil // skip path with fewer components
Expand Down
72 changes: 72 additions & 0 deletions tar_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
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.Stat(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)
if err != nil {
Expand Down Expand Up @@ -47,6 +57,68 @@ 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)
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", "symlinkvehicle.txt"))
}

func createSymlinkPathTraversalSample(t *testing.T, archivePath string) {
t.Helper()

type tarinfo struct {
Name string
Link string
Body string
Type byte
}

var infos = []tarinfo{
{"symlinkvehicle.txt", "./../target", "", tar.TypeSymlink},
{"symlinkvehicle.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