Skip to content

Commit 82ca88a

Browse files
authored
Fix tar path traversal through symlinks (#1)
* 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>
1 parent cc194d2 commit 82ca88a

File tree

2 files changed

+106
-1
lines changed

2 files changed

+106
-1
lines changed

tar.go

+18
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,24 @@ func (t *Tar) untarNext(destination string) error {
238238
return fmt.Errorf("checking path traversal attempt: %v", errPath)
239239
}
240240

241+
switch header.Typeflag {
242+
case tar.TypeSymlink, tar.TypeLink:
243+
// this covers cases when the link is an absolute path to a file outside the destination folder
244+
if filepath.IsAbs(header.Linkname) {
245+
errPath := &IllegalPathError{AbsolutePath: "", Filename: header.Linkname}
246+
return fmt.Errorf("absolute path for symlink destination not allowed: %w", errPath)
247+
}
248+
249+
// though we've already checked the name for possible path traversals, it is possible
250+
// to write content though a symlink to a path outside of the destination folder
251+
// with multiple header entries. We should consider any symlink or hardlink that points
252+
// to outside of the destination folder to be a possible path traversal attack.
253+
errPath = t.CheckPath(destination, header.Linkname)
254+
if errPath != nil {
255+
return fmt.Errorf("checking path traversal attempt in symlink: %w", errPath)
256+
}
257+
}
258+
241259
if t.StripComponents > 0 {
242260
if strings.Count(header.Name, "/") < t.StripComponents {
243261
return nil // skip path with fewer components

tar_test.go

+88-1
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,26 @@
11
package archiver_test
22

33
import (
4+
"archive/tar"
5+
"bytes"
46
"io/ioutil"
57
"os"
68
"path"
9+
"path/filepath"
710
"testing"
811

912
"github.com/mholt/archiver/v3"
1013
)
1114

15+
func requireDoesNotExist(t *testing.T, path string) {
16+
_, err := os.Lstat(path)
17+
if err == nil {
18+
t.Fatalf("'%s' expected to not exist", path)
19+
}
20+
}
21+
1222
func requireRegularFile(t *testing.T, path string) os.FileInfo {
13-
fileInfo, err := os.Stat(path)
23+
fileInfo, err := os.Lstat(path)
1424
if err != nil {
1525
t.Fatalf("fileInfo on '%s': %v", path, err)
1626
}
@@ -47,6 +57,83 @@ func TestDefaultTar_Unarchive_HardlinkSuccess(t *testing.T) {
4757
assertSameFile(t, fileaInfo, filebInfo)
4858
}
4959

60+
func TestDefaultTar_Unarchive_SymlinkPathTraversal(t *testing.T) {
61+
tmp := t.TempDir()
62+
source := filepath.Join(tmp, "source.tar")
63+
createSymlinkPathTraversalSample(t, source, "./../target")
64+
destination := filepath.Join(tmp, "destination")
65+
66+
err := archiver.DefaultTar.Unarchive(source, destination)
67+
if err != nil {
68+
t.Fatalf("unarchiving '%s' to '%s': %v", source, destination, err)
69+
}
70+
71+
requireDoesNotExist(t, filepath.Join(tmp, "target"))
72+
requireRegularFile(t, filepath.Join(tmp, "destination", "duplicatedentry.txt"))
73+
}
74+
75+
func TestDefaultTar_Unarchive_SymlinkPathTraversal_AbsLinkDestination(t *testing.T) {
76+
tmp := t.TempDir()
77+
source := filepath.Join(tmp, "source.tar")
78+
createSymlinkPathTraversalSample(t, source, "/tmp/thing")
79+
destination := filepath.Join(tmp, "destination")
80+
81+
err := archiver.DefaultTar.Unarchive(source, destination)
82+
if err != nil {
83+
t.Fatalf("unarchiving '%s' to '%s': %v", source, destination, err)
84+
}
85+
86+
requireDoesNotExist(t, "/tmp/thing")
87+
requireRegularFile(t, filepath.Join(tmp, "destination", "duplicatedentry.txt"))
88+
}
89+
90+
func createSymlinkPathTraversalSample(t *testing.T, archivePath string, linkPath string) {
91+
t.Helper()
92+
93+
type tarinfo struct {
94+
Name string
95+
Link string
96+
Body string
97+
Type byte
98+
}
99+
100+
var infos = []tarinfo{
101+
{"duplicatedentry.txt", linkPath, "", tar.TypeSymlink},
102+
{"duplicatedentry.txt", "", "content modified!", tar.TypeReg},
103+
}
104+
105+
var buf bytes.Buffer
106+
var tw = tar.NewWriter(&buf)
107+
for _, ti := range infos {
108+
hdr := &tar.Header{
109+
Name: ti.Name,
110+
Mode: 0600,
111+
Linkname: ti.Link,
112+
Typeflag: ti.Type,
113+
Size: int64(len(ti.Body)),
114+
}
115+
if err := tw.WriteHeader(hdr); err != nil {
116+
t.Fatal("Writing header: ", err)
117+
}
118+
if _, err := tw.Write([]byte(ti.Body)); err != nil {
119+
t.Fatal("Writing body: ", err)
120+
}
121+
}
122+
123+
f, err := os.Create(archivePath)
124+
if err != nil {
125+
t.Fatal(err)
126+
}
127+
_, err = f.Write(buf.Bytes())
128+
if err != nil {
129+
t.Fatal(err)
130+
}
131+
132+
if err := f.Close(); err != nil {
133+
t.Fatal(err)
134+
}
135+
}
136+
50137
func TestDefaultTar_Extract_HardlinkSuccess(t *testing.T) {
51138
source := "testdata/gnu-hardlinks.tar"
52139

0 commit comments

Comments
 (0)