Skip to content

Commit 8d200a6

Browse files
cmagliecmaglie
committed
Fixed symlink extraction handlink
1 parent 519326b commit 8d200a6

3 files changed

Lines changed: 208 additions & 29 deletions

File tree

‎extractor.go‎

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,17 +240,56 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
240240
}
241241
}
242242

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+
var placeholders []*link
252+
// remover, removeSupported := e.FS.(FSRemover)
253+
243254
for _, symlink := range symlinks {
244255
select {
245256
case <-ctx.Done():
246257
return errors.New("interrupted")
247258
default:
248259
}
260+
261+
// If the symlink pointer is clean make the symlink straighaway
262+
if clean := filepath.Clean(symlink.Name); !strings.Contains(clean, "..") && !filepath.IsAbs(clean) {
263+
_ = e.FS.Remove(symlink.Path)
264+
if err := e.FS.Symlink(symlink.Name, symlink.Path); err != nil {
265+
return errors.Annotatef(err, "Create link %s", symlink.Path)
266+
}
267+
} else {
268+
// Otherwise make a placeholder and replace it after unpacking everything
269+
_ = e.FS.Remove(symlink.Path)
270+
f, err := e.FS.OpenFile(symlink.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, os.FileMode(0666))
271+
if err != nil {
272+
return fmt.Errorf("creating symlink placeholder %s: %w", symlink.Path, err)
273+
}
274+
if err := f.Close(); err != nil {
275+
return fmt.Errorf("creating symlink placeholder %s: %w", symlink.Path, err)
276+
}
277+
placeholders = append(placeholders, symlink)
278+
}
279+
}
280+
281+
for _, symlink := range placeholders {
282+
select {
283+
case <-ctx.Done():
284+
return errors.New("interrupted")
285+
default:
286+
}
249287
_ = e.FS.Remove(symlink.Path)
250288
if err := e.FS.Symlink(symlink.Name, symlink.Path); err != nil {
251289
return errors.Annotatef(err, "Create link %s", symlink.Path)
252290
}
253291
}
292+
254293
return nil
255294
}
256295

@@ -343,17 +382,8 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
343382
}
344383
}
345384

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

359389
return nil

‎extractor_test.go‎

Lines changed: 124 additions & 0 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"
@@ -113,6 +115,128 @@ func TestZipSlipHardening(t *testing.T) {
113115
})
114116
}
115117

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)
139+
}
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.NoError(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
216+
require.FileExists(t, targetDir.Join("tmp", "sym").String())
217+
})
218+
219+
t.Run("TarWithSymlinkToExternalPathWithoutMazing", func(t *testing.T) {
220+
// Create target dir
221+
tmp := mkTempDir(t)
222+
targetDir := tmp.Join("test")
223+
require.NoError(t, targetDir.Mkdir())
224+
225+
// Make a tar archive with valid symlink maze
226+
outputTar := bytes.NewBuffer(nil)
227+
tw := tar.NewWriter(outputTar)
228+
require.NoError(t, tw.WriteHeader(&tar.Header{Mode: 0o0777, Typeflag: tar.TypeDir, Name: "tmp"}))
229+
addTarSymlink(t, tw, "aaa", "../tmp")
230+
require.NoError(t, tw.Close())
231+
232+
extractor := extract.Extractor{FS: &LoggingFS{}}
233+
require.NoError(t, extractor.Tar(context.Background(), outputTar, targetDir.String(), nil))
234+
st, err := targetDir.Join("aaa").Lstat()
235+
require.NoError(t, err)
236+
require.Equal(t, "aaa", st.Name())
237+
})
238+
}
239+
116240
// MockDisk is a disk that chroots to a directory
117241
type MockDisk struct {
118242
Base string

‎loggingfs_test.go‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,59 +17,84 @@ type LoggedOp struct {
1717
OldPath string
1818
Mode os.FileMode
1919
Flags int
20+
Err error
2021
}
2122

2223
func (op *LoggedOp) String() string {
24+
res := ""
2325
switch op.Op {
2426
case "link":
25-
return fmt.Sprintf("link %s -> %s", op.Path, op.OldPath)
27+
res += fmt.Sprintf("link %s -> %s", op.Path, op.OldPath)
2628
case "symlink":
27-
return fmt.Sprintf("symlink %s -> %s", op.Path, op.OldPath)
29+
res += fmt.Sprintf("symlink %s -> %s", op.Path, op.OldPath)
2830
case "mkdirall":
29-
return fmt.Sprintf("mkdirall %v %s", op.Mode, op.Path)
31+
res += fmt.Sprintf("mkdirall %v %s", op.Mode, op.Path)
3032
case "open":
31-
return fmt.Sprintf("open %v %s (flags=%04x)", op.Mode, op.Path, op.Flags)
33+
res += fmt.Sprintf("open %v %s (flags=%04x)", op.Mode, op.Path, op.Flags)
3234
case "remove":
33-
return fmt.Sprintf("remove %v", op.Path)
35+
res += fmt.Sprintf("remove %v", op.Path)
36+
default:
37+
panic("unknown LoggedOP " + op.Op)
38+
}
39+
if op.Err != nil {
40+
res += " error: " + op.Err.Error()
41+
} else {
42+
res += " success"
3443
}
35-
panic("unknown LoggedOP " + op.Op)
44+
return res
3645
}
3746

3847
func (m *LoggingFS) Link(oldname, newname string) error {
39-
m.Journal = append(m.Journal, &LoggedOp{
48+
err := os.Link(oldname, newname)
49+
op := &LoggedOp{
4050
Op: "link",
4151
OldPath: oldname,
4252
Path: newname,
43-
})
44-
return os.Link(oldname, newname)
53+
Err: err,
54+
}
55+
m.Journal = append(m.Journal, op)
56+
fmt.Println("FS>", op)
57+
return err
4558
}
4659

4760
func (m *LoggingFS) MkdirAll(path string, perm os.FileMode) error {
48-
m.Journal = append(m.Journal, &LoggedOp{
61+
err := os.MkdirAll(path, perm)
62+
op := &LoggedOp{
4963
Op: "mkdirall",
5064
Path: path,
5165
Mode: perm,
52-
})
53-
return os.MkdirAll(path, perm)
66+
Err: err,
67+
}
68+
m.Journal = append(m.Journal, op)
69+
fmt.Println("FS>", op)
70+
return err
5471
}
5572

5673
func (m *LoggingFS) Symlink(oldname, newname string) error {
57-
m.Journal = append(m.Journal, &LoggedOp{
74+
err := os.Symlink(oldname, newname)
75+
op := &LoggedOp{
5876
Op: "symlink",
5977
OldPath: oldname,
6078
Path: newname,
61-
})
62-
return os.Symlink(oldname, newname)
79+
Err: err,
80+
}
81+
m.Journal = append(m.Journal, op)
82+
fmt.Println("FS>", op)
83+
return err
6384
}
6485

6586
func (m *LoggingFS) OpenFile(name string, flags int, perm os.FileMode) (*os.File, error) {
66-
m.Journal = append(m.Journal, &LoggedOp{
87+
f, err := os.OpenFile(name, flags, perm)
88+
op := &LoggedOp{
6789
Op: "open",
6890
Path: name,
6991
Mode: perm,
7092
Flags: flags,
71-
})
72-
return os.OpenFile(os.DevNull, flags, perm)
93+
Err: err,
94+
}
95+
m.Journal = append(m.Journal, op)
96+
fmt.Println("FS>", op)
97+
return f, err
7398
}
7499

75100
func (m *LoggingFS) Remove(path string) error {

0 commit comments

Comments
 (0)