diff --git a/internal/update/extract.go b/internal/update/extract.go index ce0a5acbf..84b86ecef 100644 --- a/internal/update/extract.go +++ b/internal/update/extract.go @@ -53,6 +53,23 @@ func extractTarGz(archivePath string, destDir string) error { if err := os.MkdirAll(target, 0o755); err != nil { return err } + case tar.TypeSymlink: + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + return err + } + if filepath.IsAbs(header.Linkname) { + return fmt.Errorf("absolute symlink targets are not supported: %s -> %s", header.Name, header.Linkname) + } + // Verify that the symlink target, when resolved, does not escape destDir. + resolvedTarget := filepath.Join(filepath.Dir(target), header.Linkname) + destDirClean := filepath.Clean(destDir) + if !strings.HasPrefix(resolvedTarget, destDirClean+string(os.PathSeparator)) && resolvedTarget != destDirClean { + return fmt.Errorf("archive symlink target escapes destination: %s -> %s", header.Name, header.Linkname) + } + _ = os.Remove(target) + if err := os.Symlink(header.Linkname, target); err != nil { + return err + } case tar.TypeReg: if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { return err @@ -90,8 +107,13 @@ func extractZip(archivePath string, destDir string) error { // Release archives only ever contain regular files and directories; // reject anything else (symlinks, devices) rather than silently write // the link-target string (or other special content) out as an - // ordinary file — mirrors extractTarGz's rejection of non-regular tar - // entries. + // ordinary file. Unlike extractTarGz, zip symlinks stay rejected: the + // .zip path is the Windows release archive format, where + // filepath.IsAbs does not reliably reject a slash-rooted target like + // "/some/other/path" (Windows absolute paths need a drive letter or + // UNC prefix), and there is no current use case for symlinks in a + // Windows archive. The npm shim symlinks this feature exists for are + // packaged in the Unix/macOS .tar.gz archives, handled above. if !entry.Mode().IsRegular() { return fmt.Errorf("unsupported archive entry type for %s", entry.Name) } diff --git a/internal/update/extract_test.go b/internal/update/extract_test.go index f8aaa8239..1742866ea 100644 --- a/internal/update/extract_test.go +++ b/internal/update/extract_test.go @@ -215,3 +215,96 @@ func TestFindByBasenameSearchesRecursively(t *testing.T) { t.Fatalf("findByBasename = %q, want empty", notFound) } } + +func symlinksSupported(t *testing.T) bool { + dir := t.TempDir() + err := os.Symlink("target", filepath.Join(dir, "link")) + return err == nil +} + +func TestExtractTarGzAllowsSafeSymlink(t *testing.T) { + if !symlinksSupported(t) { + t.Skip("symlinks not supported") + } + dir := t.TempDir() + archivePath := filepath.Join(dir, "archive.tar.gz") + + file, err := os.Create(archivePath) + if err != nil { + t.Fatalf("Create: %v", err) + } + gw := gzip.NewWriter(file) + tw := tar.NewWriter(gw) + + h1 := &tar.Header{ + Name: "link.txt", + Typeflag: tar.TypeSymlink, + Linkname: "target.txt", + } + if err := tw.WriteHeader(h1); err != nil { + t.Fatalf("WriteHeader link: %v", err) + } + h2 := &tar.Header{ + Name: "target.txt", + Typeflag: tar.TypeReg, + Mode: 0o644, + Size: 12, + } + if err := tw.WriteHeader(h2); err != nil { + t.Fatalf("WriteHeader target: %v", err) + } + if _, err := tw.Write([]byte("hello symbol")); err != nil { + t.Fatalf("Write target: %v", err) + } + + tw.Close() + gw.Close() + file.Close() + + destDir := filepath.Join(dir, "extracted") + if err := extractArchive(archivePath, destDir); err != nil { + t.Fatalf("extractArchive: %v", err) + } + + targetPath := filepath.Join(destDir, "link.txt") + gotLink, err := os.Readlink(targetPath) + if err != nil { + t.Fatalf("Readlink: %v", err) + } + if gotLink != "target.txt" { + t.Fatalf("link target = %q, want %q", gotLink, "target.txt") + } +} + +func TestExtractTarGzRejectsEscapingSymlink(t *testing.T) { + if !symlinksSupported(t) { + t.Skip("symlinks not supported") + } + dir := t.TempDir() + archivePath := filepath.Join(dir, "archive.tar.gz") + + file, err := os.Create(archivePath) + if err != nil { + t.Fatalf("Create: %v", err) + } + gw := gzip.NewWriter(file) + tw := tar.NewWriter(gw) + + h1 := &tar.Header{ + Name: "link.txt", + Typeflag: tar.TypeSymlink, + Linkname: "../../../outside.txt", + } + if err := tw.WriteHeader(h1); err != nil { + t.Fatalf("WriteHeader: %v", err) + } + + tw.Close() + gw.Close() + file.Close() + + destDir := filepath.Join(dir, "extracted") + if err := extractArchive(archivePath, destDir); err == nil { + t.Fatal("expected error extracting escaping symlink") + } +}