From 26536b2c369ea51391c3aec06a7ebe6748d738f8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 19 Jun 2026 12:08:34 +0100 Subject: [PATCH] fix(desktop): confine snapshot file reads --- CHANGELOG.md | 4 +++ internal/slackdesktop/desktop.go | 47 ++++++++++++++++++++------- internal/slackdesktop/desktop_test.go | 8 +++-- 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 724aab8..2ce3e1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 0.7.3 - Unreleased +### Fixes + +- Confined Slack Desktop snapshot reads to the discovered profile root and rejected symlink or special-file entries. + ### Maintenance - Updated crawlkit through 0.12.2 for shared runtime hardening, SQLite 1.52, and absolute Windows database paths. diff --git a/internal/slackdesktop/desktop.go b/internal/slackdesktop/desktop.go index b67fa0e..75ec624 100644 --- a/internal/slackdesktop/desktop.go +++ b/internal/slackdesktop/desktop.go @@ -273,6 +273,16 @@ func SnapshotPath(path string) (snapshot Snapshot, err error) { if err := os.MkdirAll(target, 0o750); err != nil { return Snapshot{}, err } + sourceRoot, err := os.OpenRoot(path) + if err != nil { + return Snapshot{}, err + } + defer func() { _ = sourceRoot.Close() }() + targetRoot, err := os.OpenRoot(target) + if err != nil { + return Snapshot{}, err + } + defer func() { _ = targetRoot.Close() }() copyTargets := []string{ rootStateFile, @@ -282,15 +292,14 @@ func SnapshotPath(path string) (snapshot Snapshot, err error) { indexedDBBlobDir, } for _, relative := range copyTargets { - src := filepath.Join(path, filepath.FromSlash(relative)) - if _, err := os.Stat(src); err != nil { + relative = filepath.FromSlash(relative) + if _, err := sourceRoot.Lstat(relative); err != nil { if os.IsNotExist(err) { continue } return Snapshot{}, err } - dst := filepath.Join(target, filepath.FromSlash(relative)) - if err := copyPath(src, dst); err != nil { + if err := copyPath(sourceRoot, targetRoot, relative); err != nil { return Snapshot{}, err } } @@ -1283,34 +1292,48 @@ func intString(value int) string { return string(data) } -func copyPath(src string, dst string) error { - info, err := os.Stat(src) +func copyPath(srcRoot *os.Root, dstRoot *os.Root, relative string) error { + info, err := srcRoot.Lstat(relative) if err != nil { return err } + if info.Mode()&os.ModeSymlink != 0 { + return errors.New("desktop snapshot source contains a symbolic link") + } if info.IsDir() { - if err := os.MkdirAll(dst, info.Mode()); err != nil { + if err := dstRoot.MkdirAll(relative, info.Mode().Perm()); err != nil { return err } - entries, err := os.ReadDir(src) + dir, err := srcRoot.Open(relative) if err != nil { return err } + entries, readErr := dir.ReadDir(-1) + closeErr := dir.Close() + if readErr != nil { + return readErr + } + if closeErr != nil { + return closeErr + } for _, entry := range entries { - if err := copyPath(filepath.Join(src, entry.Name()), filepath.Join(dst, entry.Name())); err != nil { + if err := copyPath(srcRoot, dstRoot, filepath.Join(relative, entry.Name())); err != nil { return err } } return nil } - if err := os.MkdirAll(filepath.Dir(dst), 0o750); err != nil { + if !info.Mode().IsRegular() { + return errors.New("desktop snapshot source contains a special file") + } + if err := dstRoot.MkdirAll(filepath.Dir(relative), 0o750); err != nil { return err } - data, err := os.ReadFile(src) //nolint:gosec // Snapshot copy reads from discovered Slack desktop paths. + data, err := srcRoot.ReadFile(relative) if err != nil { return err } - return os.WriteFile(dst, data, info.Mode()) + return dstRoot.WriteFile(relative, data, info.Mode().Perm()) } func cleanKey(key []byte) string { diff --git a/internal/slackdesktop/desktop_test.go b/internal/slackdesktop/desktop_test.go index a12619d..87e232c 100644 --- a/internal/slackdesktop/desktop_test.go +++ b/internal/slackdesktop/desktop_test.go @@ -298,11 +298,13 @@ func TestDiscoverEmptyPathIsUnavailable(t *testing.T) { require.Empty(t, source.Path) } -func TestSnapshotPathRemovesPartialSnapshotOnError(t *testing.T) { +func TestSnapshotPathRejectsEscapingSymlinkAndRemovesPartialSnapshot(t *testing.T) { root := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(root, "storage"), 0o750)) - loopPath := filepath.Join(root, "storage", "root-state.json") - if err := os.Symlink("root-state.json", loopPath); err != nil { + external := filepath.Join(t.TempDir(), "outside.json") + require.NoError(t, os.WriteFile(external, []byte(`{"outside":true}`), 0o600)) + symlinkPath := filepath.Join(root, "storage", "root-state.json") + if err := os.Symlink(external, symlinkPath); err != nil { t.Skipf("symlink unavailable: %v", err) }