Skip to content

Commit efa640e

Browse files
committed
fix(update): resolve intermediate symlink parents and cap Root link depth
Walk each path component from the physical parent, reject a ninth link, and look up release binaries through OpenRoot as regular files only.
1 parent 67f151f commit efa640e

2 files changed

Lines changed: 93 additions & 38 deletions

File tree

‎internal/update/extract.go‎

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -216,39 +216,17 @@ func validateSymlinkTarget(root *os.Root, parentRel, linkTarget string) error {
216216
return err
217217
}
218218

219+
const maxRootSymlinks = 8
220+
219221
func followUnderRoot(root *os.Root, rel string, depth int) (string, error) {
220-
if depth > 255 {
221-
return "", fmt.Errorf("archive symlink nest exceeds limit")
222-
}
223222
if rel == "" || rel == "." {
224223
return ".", nil
225224
}
226-
info, err := root.Lstat(rel)
227-
if err != nil {
228-
if os.IsNotExist(err) {
229-
return rel, nil
230-
}
231-
return "", err
232-
}
233-
if info.Mode()&os.ModeSymlink == 0 {
234-
return rel, nil
235-
}
236-
tgt, err := root.Readlink(rel)
237-
if err != nil {
238-
return "", err
239-
}
240-
if filepath.IsAbs(tgt) || strings.HasPrefix(tgt, "/") {
241-
return "", fmt.Errorf("archive symlink %s has absolute target: %s", rel, tgt)
242-
}
243-
parent := filepath.Dir(rel)
244-
if parent == "" {
245-
parent = "."
246-
}
247-
return walkUnderRoot(root, parent, tgt, depth+1)
225+
return walkUnderRoot(root, ".", rel, depth)
248226
}
249227

250228
func walkUnderRoot(root *os.Root, base, linkTarget string, depth int) (string, error) {
251-
if depth > 255 {
229+
if depth > maxRootSymlinks {
252230
return "", fmt.Errorf("archive symlink nest exceeds limit")
253231
}
254232
current := base
@@ -285,7 +263,15 @@ func walkUnderRoot(root *os.Root, base, linkTarget string, depth int) (string, e
285263
current = next
286264
continue
287265
}
288-
followed, err := followUnderRoot(root, next, depth+1)
266+
tgt, err := root.Readlink(next)
267+
if err != nil {
268+
return "", err
269+
}
270+
if filepath.IsAbs(tgt) || strings.HasPrefix(tgt, "/") || strings.HasPrefix(tgt, "\\") ||
271+
filepath.VolumeName(tgt) != "" || strings.Contains(tgt, ":") {
272+
return "", fmt.Errorf("archive symlink %s has absolute target: %s", next, tgt)
273+
}
274+
followed, err := walkUnderRoot(root, current, tgt, depth+1)
289275
if err != nil {
290276
return "", err
291277
}
@@ -299,16 +285,25 @@ func walkUnderRoot(root *os.Root, base, linkTarget string, depth int) (string, e
299285
// helper binaries nested under archive subdirectories (e.g. helpers/) are
300286
// still found.
301287
func findByBasename(root string, name string) (string, error) {
288+
destRoot, err := os.OpenRoot(root)
289+
if err != nil {
290+
return "", err
291+
}
292+
defer destRoot.Close()
302293
var found string
303-
err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, walkErr error) error {
294+
err = fs.WalkDir(destRoot.FS(), ".", func(path string, entry fs.DirEntry, walkErr error) error {
304295
if walkErr != nil {
305296
return walkErr
306297
}
307298
if found != "" {
308299
return fs.SkipAll
309300
}
310-
if !entry.IsDir() && entry.Name() == name {
311-
found = path
301+
if entry.Type().IsRegular() && entry.Name() == name {
302+
if path == "." {
303+
found = filepath.Join(root, name)
304+
} else {
305+
found = filepath.Join(root, filepath.FromSlash(path))
306+
}
312307
}
313308
return nil
314309
})

‎internal/update/extract_test.go‎

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"archive/tar"
55
"archive/zip"
66
"compress/gzip"
7+
"fmt"
78
"os"
89
"path/filepath"
910
"testing"
@@ -258,21 +259,23 @@ func TestExtractTarGzAllowsSafeSymlink(t *testing.T) {
258259
}
259260

260261
// Cover a linked subdirectory with a file extracted through the link.
262+
// Windows Root.Symlink needs the directory target to exist first so the
263+
// link is created as a directory junction rather than a file symlink.
261264
h3 := &tar.Header{
262-
Name: "sublink",
263-
Typeflag: tar.TypeSymlink,
264-
Linkname: "subdir",
265+
Name: "subdir",
266+
Typeflag: tar.TypeDir,
267+
Mode: 0o755,
265268
}
266269
if err := tw.WriteHeader(h3); err != nil {
267-
t.Fatalf("WriteHeader sublink: %v", err)
270+
t.Fatalf("WriteHeader subdir: %v", err)
268271
}
269272
h4 := &tar.Header{
270-
Name: "subdir",
271-
Typeflag: tar.TypeDir,
272-
Mode: 0o755,
273+
Name: "sublink",
274+
Typeflag: tar.TypeSymlink,
275+
Linkname: "subdir",
273276
}
274277
if err := tw.WriteHeader(h4); err != nil {
275-
t.Fatalf("WriteHeader subdir: %v", err)
278+
t.Fatalf("WriteHeader sublink: %v", err)
276279
}
277280
h5 := &tar.Header{
278281
Name: "sublink/nested.txt",
@@ -532,3 +535,60 @@ func TestExtractTarGzRejectsSymlinkThroughSymlinkParentOutsideTarget(t *testing.
532535
t.Fatalf("file outside destDir was accessed/created: %v", err)
533536
}
534537
}
538+
539+
func TestExtractTarGzRejectsIntermediateDirSymlinkEscape(t *testing.T) {
540+
if !symlinksSupported(t) {
541+
t.Skip("symlinks not supported")
542+
}
543+
dir := t.TempDir()
544+
destDir := filepath.Join(dir, "extracted")
545+
outside := filepath.Join(dir, "archive.tar.gz.outside")
546+
if err := os.WriteFile(outside, []byte("secret"), 0o644); err != nil {
547+
t.Fatal(err)
548+
}
549+
archivePath := filepath.Join(dir, "archive.tar.gz")
550+
writeTestTarGzEntries(t, archivePath, []testTarEntry{
551+
{name: "d", typeflag: tar.TypeSymlink, linkname: "."},
552+
{name: "d/a", typeflag: tar.TypeDir},
553+
{name: "d/a/zero", typeflag: tar.TypeSymlink, linkname: "../../archive.tar.gz.outside"},
554+
})
555+
if err := extractArchive(archivePath, destDir); err == nil {
556+
t.Fatal("expected reject of ../../ escape through intermediate dir symlink")
557+
}
558+
if _, err := os.Lstat(filepath.Join(destDir, "d", "a", "zero")); err == nil {
559+
t.Fatal("escaping link must not be created")
560+
}
561+
}
562+
563+
func TestFollowUnderRootRejectsNinthLink(t *testing.T) {
564+
if !symlinksSupported(t) {
565+
t.Skip("symlinks not supported")
566+
}
567+
dir := t.TempDir()
568+
root, err := os.OpenRoot(dir)
569+
if err != nil {
570+
t.Fatal(err)
571+
}
572+
defer root.Close()
573+
if err := os.Mkdir(filepath.Join(dir, "leaf"), 0o755); err != nil {
574+
t.Fatal(err)
575+
}
576+
prev := "leaf"
577+
for i := 0; i < maxRootSymlinks; i++ {
578+
name := fmt.Sprintf("l%d", i)
579+
if err := os.Symlink(prev, filepath.Join(dir, name)); err != nil {
580+
t.Fatal(err)
581+
}
582+
prev = name
583+
}
584+
if _, err := followUnderRoot(root, prev, 0); err != nil {
585+
t.Fatalf("eight links must resolve: %v", err)
586+
}
587+
ninth := "l8"
588+
if err := os.Symlink(prev, filepath.Join(dir, ninth)); err != nil {
589+
t.Fatal(err)
590+
}
591+
if _, err := followUnderRoot(root, ninth, 0); err == nil {
592+
t.Fatal("ninth link must be rejected")
593+
}
594+
}

0 commit comments

Comments
 (0)