Skip to content

Commit 4a98568

Browse files
cmagliecmaglie
authored
Merge commit from fork
More zipslip cases
2 parents 685dc25 + b347945 commit 4a98568

5 files changed

Lines changed: 265 additions & 35 deletions

File tree

‎evil_generator/main.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ func main() {
2222
log.Fatalf("Output path %s is not a directory", outputDir)
2323
}
2424

25+
generateEvilZipSlip(outputDir)
26+
generateEvilSymLinkPathTraversalTar(outputDir)
27+
}
28+
29+
func generateEvilZipSlip(outputDir *paths.Path) {
2530
evilPathTraversalFiles := []string{
2631
"..",
2732
"../../../../../../../../../../../../../../../../../../../../tmp/evil.txt",
@@ -104,3 +109,21 @@ func main() {
104109
}
105110
}
106111
}
112+
113+
func generateEvilSymLinkPathTraversalTar(outputDir *paths.Path) {
114+
outputTarFile, err := outputDir.Join("evil-link-traversal.tar").Create()
115+
if err != nil {
116+
log.Fatal(err)
117+
}
118+
defer outputTarFile.Close()
119+
120+
tw := tar.NewWriter(outputTarFile)
121+
defer tw.Close()
122+
123+
if err := tw.WriteHeader(&tar.Header{
124+
Name: "leak", Linkname: "../../../../../../../../../../../../../../../tmp/something-important",
125+
Mode: 0o0777, Size: 0, Typeflag: tar.TypeLink,
126+
}); err != nil {
127+
log.Fatal(err)
128+
}
129+
}

‎extractor.go‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,10 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
217217
name = rename(name)
218218
}
219219

220-
name = filepath.Join(location, name)
220+
name, err = safeJoin(location, name)
221+
if err != nil {
222+
continue
223+
}
221224
links = append(links, &link{Path: path, Name: name})
222225
case tar.TypeSymlink:
223226
symlinks = append(symlinks, &link{Path: path, Name: header.Linkname})
@@ -237,6 +240,32 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
237240
}
238241
}
239242

243+
if err := e.extractSymlinks(ctx, symlinks); err != nil {
244+
return err
245+
}
246+
247+
return nil
248+
}
249+
250+
func (e *Extractor) extractSymlinks(ctx context.Context, symlinks []*link) error {
251+
for _, symlink := range symlinks {
252+
select {
253+
case <-ctx.Done():
254+
return errors.New("interrupted")
255+
default:
256+
}
257+
258+
// Make a placeholder and replace it after unpacking everything
259+
_ = e.FS.Remove(symlink.Path)
260+
f, err := e.FS.OpenFile(symlink.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, os.FileMode(0666))
261+
if err != nil {
262+
return fmt.Errorf("creating symlink placeholder %s: %w", symlink.Path, err)
263+
}
264+
if err := f.Close(); err != nil {
265+
return fmt.Errorf("creating symlink placeholder %s: %w", symlink.Path, err)
266+
}
267+
}
268+
240269
for _, symlink := range symlinks {
241270
select {
242271
case <-ctx.Done():
@@ -248,6 +277,7 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
248277
return errors.Annotatef(err, "Create link %s", symlink.Path)
249278
}
250279
}
280+
251281
return nil
252282
}
253283

@@ -340,17 +370,8 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
340370
}
341371
}
342372

343-
// Now we make another pass creating the links
344-
for _, link := range links {
345-
select {
346-
case <-ctx.Done():
347-
return errors.New("interrupted")
348-
default:
349-
}
350-
_ = e.FS.Remove(link.Path)
351-
if err := e.FS.Symlink(link.Name, link.Path); err != nil {
352-
return errors.Annotatef(err, "Create link %s", link.Path)
353-
}
373+
if err := e.extractSymlinks(ctx, links); err != nil {
374+
return err
354375
}
355376

356377
return nil

‎extractor_test.go‎

Lines changed: 166 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package extract_test
22

33
import (
4+
"archive/tar"
5+
"archive/zip"
46
"bytes"
57
"context"
68
"fmt"
@@ -65,7 +67,7 @@ func testArchive(t *testing.T, archivePath *paths.Path) {
6567
}
6668

6769
func TestZipSlipHardening(t *testing.T) {
68-
{
70+
t.Run("ZipTraversal", func(t *testing.T) {
6971
logger := &LoggingFS{}
7072
extractor := extract.Extractor{FS: logger}
7173
data, err := os.Open("testdata/zipslip/evil.zip")
@@ -74,8 +76,9 @@ func TestZipSlipHardening(t *testing.T) {
7476
require.NoError(t, data.Close())
7577
fmt.Print(logger)
7678
require.Empty(t, logger.Journal)
77-
}
78-
{
79+
})
80+
81+
t.Run("TarTraversal", func(t *testing.T) {
7982
logger := &LoggingFS{}
8083
extractor := extract.Extractor{FS: logger}
8184
data, err := os.Open("testdata/zipslip/evil.tar")
@@ -84,9 +87,23 @@ func TestZipSlipHardening(t *testing.T) {
8487
require.NoError(t, data.Close())
8588
fmt.Print(logger)
8689
require.Empty(t, logger.Journal)
87-
}
90+
})
91+
92+
t.Run("TarLinkTraversal", func(t *testing.T) {
93+
logger := &LoggingFS{}
94+
extractor := extract.Extractor{FS: logger}
95+
data, err := os.Open("testdata/zipslip/evil-link-traversal.tar")
96+
require.NoError(t, err)
97+
require.NoError(t, extractor.Tar(context.Background(), data, "/tmp/test", nil))
98+
require.NoError(t, data.Close())
99+
fmt.Print(logger)
100+
require.Empty(t, logger.Journal)
101+
})
88102

89-
if runtime.GOOS == "windows" {
103+
t.Run("WindowsTarTraversal", func(t *testing.T) {
104+
if runtime.GOOS != "windows" {
105+
t.Skip("Skipped on non-Windows host")
106+
}
90107
logger := &LoggingFS{}
91108
extractor := extract.Extractor{FS: logger}
92109
data, err := os.Open("testdata/zipslip/evil-win.tar")
@@ -95,7 +112,151 @@ func TestZipSlipHardening(t *testing.T) {
95112
require.NoError(t, data.Close())
96113
fmt.Print(logger)
97114
require.Empty(t, logger.Journal)
115+
})
116+
}
117+
118+
func mkTempDir(t *testing.T) *paths.Path {
119+
tmp, err := paths.MkTempDir("", "test")
120+
require.NoError(t, err)
121+
t.Cleanup(func() { tmp.RemoveAll() })
122+
return tmp
123+
}
124+
125+
func TestSymLinkMazeHardening(t *testing.T) {
126+
addTarSymlink := func(t *testing.T, tw *tar.Writer, new, old string) {
127+
err := tw.WriteHeader(&tar.Header{
128+
Mode: 0o0777, Typeflag: tar.TypeSymlink, Name: new, Linkname: old,
129+
})
130+
require.NoError(t, err)
131+
}
132+
addZipSymlink := func(t *testing.T, zw *zip.Writer, new, old string) {
133+
h := &zip.FileHeader{Name: new, Method: zip.Deflate}
134+
h.SetMode(os.ModeSymlink)
135+
w, err := zw.CreateHeader(h)
136+
require.NoError(t, err)
137+
_, err = w.Write([]byte(old))
138+
require.NoError(t, err)
98139
}
140+
141+
t.Run("TarWithSymlinkToAbsPath", func(t *testing.T) {
142+
// Create target dir
143+
tmp := mkTempDir(t)
144+
targetDir := tmp.Join("test")
145+
require.NoError(t, targetDir.Mkdir())
146+
147+
// Make a tar archive with symlink maze
148+
outputTar := bytes.NewBuffer(nil)
149+
tw := tar.NewWriter(outputTar)
150+
addTarSymlink(t, tw, "aaa", tmp.String())
151+
addTarSymlink(t, tw, "aaa/sym", "something")
152+
require.NoError(t, tw.Close())
153+
154+
// Run extract
155+
extractor := extract.Extractor{FS: &LoggingFS{}}
156+
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
157+
require.NoFileExists(t, tmp.Join("sym").String())
158+
})
159+
160+
t.Run("ZipWithSymlinkToAbsPath", func(t *testing.T) {
161+
// Create target dir
162+
tmp := mkTempDir(t)
163+
targetDir := tmp.Join("test")
164+
require.NoError(t, targetDir.Mkdir())
165+
166+
// Make a zip archive with symlink maze
167+
outputZip := bytes.NewBuffer(nil)
168+
zw := zip.NewWriter(outputZip)
169+
addZipSymlink(t, zw, "aaa", tmp.String())
170+
addZipSymlink(t, zw, "aaa/sym", "something")
171+
require.NoError(t, zw.Close())
172+
173+
// Run extract
174+
extractor := extract.Extractor{FS: &LoggingFS{}}
175+
err := extractor.Zip(context.Background(), outputZip, targetDir.String(), nil)
176+
require.NoFileExists(t, tmp.Join("sym").String())
177+
require.Error(t, err)
178+
})
179+
180+
t.Run("TarWithSymlinkToRelativeExternalPath", func(t *testing.T) {
181+
// Create target dir
182+
tmp := mkTempDir(t)
183+
targetDir := tmp.Join("test")
184+
require.NoError(t, targetDir.Mkdir())
185+
checkDir := tmp.Join("secret")
186+
require.NoError(t, checkDir.MkdirAll())
187+
188+
// Make a tar archive with regular symlink maze
189+
outputTar := bytes.NewBuffer(nil)
190+
tw := tar.NewWriter(outputTar)
191+
addTarSymlink(t, tw, "aaa", "../secret")
192+
addTarSymlink(t, tw, "aaa/sym", "something")
193+
require.NoError(t, tw.Close())
194+
195+
extractor := extract.Extractor{FS: &LoggingFS{}}
196+
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
197+
require.NoFileExists(t, checkDir.Join("sym").String())
198+
})
199+
200+
t.Run("TarWithSymlinkToInternalPath", func(t *testing.T) {
201+
// Create target dir
202+
tmp := mkTempDir(t)
203+
targetDir := tmp.Join("test")
204+
require.NoError(t, targetDir.Mkdir())
205+
206+
// Make a tar archive with regular symlink maze
207+
outputTar := bytes.NewBuffer(nil)
208+
tw := tar.NewWriter(outputTar)
209+
require.NoError(t, tw.WriteHeader(&tar.Header{Mode: 0o0777, Typeflag: tar.TypeDir, Name: "tmp"}))
210+
addTarSymlink(t, tw, "aaa", "tmp")
211+
addTarSymlink(t, tw, "aaa/sym", "something")
212+
require.NoError(t, tw.Close())
213+
214+
extractor := extract.Extractor{FS: &LoggingFS{}}
215+
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
216+
require.NoFileExists(t, targetDir.Join("tmp", "sym").String())
217+
})
218+
219+
t.Run("TarWithDoubleSymlinkToExternalPath", func(t *testing.T) {
220+
// Create target dir
221+
tmp := mkTempDir(t)
222+
targetDir := tmp.Join("test")
223+
require.NoError(t, targetDir.Mkdir())
224+
fmt.Println("TMP:", tmp)
225+
fmt.Println("TARGET DIR:", targetDir)
226+
227+
// Make a tar archive with regular symlink maze
228+
outputTar := bytes.NewBuffer(nil)
229+
tw := tar.NewWriter(outputTar)
230+
tw.WriteHeader(&tar.Header{Name: "fake", Mode: 0777, Typeflag: tar.TypeDir})
231+
addTarSymlink(t, tw, "sym-maze", tmp.String())
232+
addTarSymlink(t, tw, "sym-maze", "fake")
233+
addTarSymlink(t, tw, "sym-maze/oops", "/tmp/something")
234+
require.NoError(t, tw.Close())
235+
236+
extractor := extract.Extractor{FS: &LoggingFS{}}
237+
require.Error(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
238+
require.NoFileExists(t, tmp.Join("oops").String())
239+
})
240+
241+
t.Run("TarWithSymlinkToExternalPathWithoutMazing", func(t *testing.T) {
242+
// Create target dir
243+
tmp := mkTempDir(t)
244+
targetDir := tmp.Join("test")
245+
require.NoError(t, targetDir.Mkdir())
246+
247+
// Make a tar archive with valid symlink maze
248+
outputTar := bytes.NewBuffer(nil)
249+
tw := tar.NewWriter(outputTar)
250+
require.NoError(t, tw.WriteHeader(&tar.Header{Mode: 0o0777, Typeflag: tar.TypeDir, Name: "tmp"}))
251+
addTarSymlink(t, tw, "aaa", "../tmp")
252+
require.NoError(t, tw.Close())
253+
254+
extractor := extract.Extractor{FS: &LoggingFS{}}
255+
require.NoError(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
256+
st, err := targetDir.Join("aaa").Lstat()
257+
require.NoError(t, err)
258+
require.Equal(t, "aaa", st.Name())
259+
})
99260
}
100261

101262
// MockDisk is a disk that chroots to a directory

0 commit comments

Comments
 (0)