andy/mapsize
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

Fix review findings: hostile snapshots, stale async results, trash safety

Browse Source 
- snapshot: depth limit (stack overflow in exporters), decompressed-size
  limit (gzip bombs), per-node size cap and saturating aggregation
- export: sanitise exclusion patterns from snapshot metadata
- tui: drop duplicate/trash/filter results that belong to a replaced
  tree; cancel duplicate search on rescan
- trash: refuse symlinked or foreign shared-filesystem trash dirs; never
  overwrite an existing trashed item
- scan: no symlink following without inode identity; correct unscanned
  count and keep real read errors after cancellation
This commit is contained in:
Andy Dixon
2026-10-02 08:46:52 +01:00
parent 11a0e401ee
commit f4ef12a0dd
10 changed files with 315 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ARCHITECTURE.md
+8 -4
View File
@@ -152,7 +152,9 @@ releases the lock between batches). This is the only lock in the data path.
own (tiny) size. `same-filesystem` / `all` follow directory symlinks; every
descended directory's `(dev, inode)` is recorded and a directory already
visited is never descended twice — loops terminate and nothing is counted
twice.
twice. On platforms without inode identity (Windows, other portable
fallbacks) loop detection is impossible, so symlinks are never followed
there and scan info says so.
* A directory whose device differs from its parent is flagged `MountPoint`.
With `--one-file-system` it is not descended. On Linux, virtual filesystems
(proc, sysfs, cgroup, debugfs, …) are never descended — `/proc/kcore` would
@@ -292,9 +294,11 @@ Filenames and snapshot contents are untrusted.
* Text is clipped by display width (grapheme clusters, East-Asian wide
characters) so labels never overflow their rectangle.
* Snapshot decoding validates magic, version, every length, every parent
reference (a parent must precede its child, so cycles and recursion are
impossible), caps decompressed size and node count, and verifies a SHA-256
trailer. Aggregates are recomputed rather than trusted.
reference (a parent must precede its child, so cycles are impossible),
depth (≤ 4096 levels, bounding recursion in exporters), per-node sizes
(≤ 1 EiB) and node count; caps the decompressed body at 64× the file size
(decompression bombs fail fast); and verifies a SHA-256 trailer.
Aggregates are recomputed with saturating arithmetic rather than trusted.
* No shell is ever invoked. "Open/reveal" and trash use direct `exec` with
argument vectors or native APIs; paths are passed as single arguments.
* `--read-only` disables every mutating action in the UI.
internal/export/export.go
+2 -1
View File
@@ -10,6 +10,7 @@ import (
"io"
"path/filepath"
"strconv"
"strings"
"time"
"github.com/andydixon/mapsize/internal/brand"
@@ -203,7 +204,7 @@ func Summary(w io.Writer, t *inventory.Tree) {
}
}
if st.Excluded > 0 {
fmt.Fprintf(w, "Excluded %s entries (%v)\n", textutil.Count(st.Excluded), st.Excludes)
fmt.Fprintf(w, "Excluded %s entries (%s)\n", textutil.Count(st.Excluded), textutil.Sanitize(strings.Join(st.Excludes, ", ")))
}
if st.SkippedMounts > 0 {
fmt.Fprintf(w, "Other FS skipped %s\n", textutil.Count(st.SkippedMounts))
internal/inventory/tree.go
+19 -8
View File
@@ -8,6 +8,7 @@
package inventory
import (
"math"
"path/filepath"
"slices"
"sync"
@@ -210,6 +211,14 @@ func (t *Tree) Propagate(id NodeID, d Delta) {
}
}
// satAdd adds non-negative int64s, saturating at the maximum.
func satAdd(a, b int64) int64 {
if b > 0 && a > math.MaxInt64-b {
return math.MaxInt64
}
return a + b
}
func addU32(a uint32, d int64) uint32 {
v := int64(a) + d
if v < 0 {
@@ -300,23 +309,25 @@ func (t *Tree) Recompute() {
*t.dirs.at(n.dir) = [NumCategories]int64{}
}
}
// Sums saturate: snapshot data is untrusted and must not wrap to
// negative totals.
for i := t.Len() - 1; i > 0; i-- {
n := t.Node(NodeID(i))
p := t.Node(n.Parent)
p.TotSize += n.TotSize
p.TotAlloc += n.TotAlloc
p.Errors += n.Errors
p.TotSize = satAdd(p.TotSize, n.TotSize)
p.TotAlloc = satAdd(p.TotAlloc, n.TotAlloc)
p.Errors = addU32(p.Errors, int64(n.Errors))
cs := t.dirs.at(p.dir)
if n.Kind == KindDir {
p.Files += n.Files
p.Dirs += n.Dirs + 1
p.Files = addU32(p.Files, int64(n.Files))
p.Dirs = addU32(p.Dirs, int64(n.Dirs)+1)
for c, v := range t.dirs.at(n.dir) {
cs[c] += v
cs[c] = satAdd(cs[c], v)
}
} else {
p.Files++
p.Files = addU32(p.Files, 1)
if n.Flags&FlagHardlinkDup == 0 {
cs[n.Cat] += n.Alloc
cs[n.Cat] = satAdd(cs[n.Cat], n.Alloc)
}
}
}
internal/platform/ops_linux.go
+42 -5
View File
@@ -60,11 +60,19 @@ func Trash(path string) error {
infoPath = rel
}
files, info := filepath.Join(trash, "files"), filepath.Join(trash, "info")
if err := os.MkdirAll(files, 0o700); err != nil {
return err
if trash == home {
if err := os.MkdirAll(trash, 0o700); err != nil {
return err
}
} else if err := ensurePrivateDir(trash, true); err != nil {
// On a shared filesystem another user could plant a symlink or a
// world-writable directory here; refuse rather than follow it.
return fmt.Errorf("trash directory %s is unsafe: %w", trash, err)
}
if err := os.MkdirAll(info, 0o700); err != nil {
return err
for _, d := range []string{files, info} {
if err := ensurePrivateDir(d, trash != home); err != nil {
return fmt.Errorf("trash directory %s is unsafe: %w", d, err)
}
}
base := filepath.Base(path)
for i := 1; i < 10000; i++ {
@@ -72,6 +80,10 @@ func Trash(path string) error {
if i > 1 {
name = fmt.Sprintf("%s.%d", base, i)
}
dst := filepath.Join(files, name)
if _, err := os.Lstat(dst); err == nil {
continue // never overwrite an earlier trashed item (even without .trashinfo)
}
ip := filepath.Join(info, name+".trashinfo")
f, err := os.OpenFile(ip, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600)
if errors.Is(err, os.ErrExist) {
@@ -87,7 +99,7 @@ func Trash(path string) error {
os.Remove(ip)
return errors.Join(werr, cerr)
}
if err := os.Rename(path, filepath.Join(files, name)); err != nil {
if err := os.Rename(path, dst); err != nil {
os.Remove(ip)
return err
}
@@ -96,6 +108,31 @@ func Trash(path string) error {
return errors.New("trash: too many items with the same name")
}
// ensurePrivateDir creates dir (mode 0700) if missing; an existing entry must
// be a real directory (not a symlink) owned by us. With strict (trash on a
// shared filesystem) it must also be inaccessible to others, as the
// freedesktop.org spec requires; home trash directories made by desktop
// environments are sometimes 0755, which is harmless there.
func ensurePrivateDir(dir string, strict bool) error {
if err := os.Mkdir(dir, 0o700); err != nil && !errors.Is(err, os.ErrExist) {
return err
}
fi, err := os.Lstat(dir)
if err != nil {
return err
}
st := fi.Sys().(*syscall.Stat_t)
switch {
case !fi.IsDir():
return errors.New("not a directory (symlink?)")
case int(st.Uid) != os.Getuid():
return errors.New("owned by another user")
case strict && fi.Mode().Perm()&0o077 != 0:
return errors.New("accessible to other users")
}
return nil
}
// mountTop returns the top directory of the filesystem containing path.
func mountTop(path string, dev uint64) string {
cur := filepath.Dir(path)
internal/platform/ops_linux_test.go
+20
View File
@@ -42,3 +42,23 @@ func TestTrashFreedesktop(t *testing.T) {
t.Fatal("relative paths must be refused")
}
}
func TestEnsurePrivateDirRejectsSymlink(t *testing.T) {
base := t.TempDir()
target := filepath.Join(base, "attacker")
os.Mkdir(target, 0o700)
link := filepath.Join(base, ".Trash-1000")
os.Symlink(target, link)
if err := ensurePrivateDir(link, false); err == nil {
t.Fatal("symlinked trash dir accepted")
}
open := filepath.Join(base, "open")
os.Mkdir(open, 0o777)
os.Chmod(open, 0o777)
if err := ensurePrivateDir(open, true); err == nil {
t.Fatal("world-accessible trash dir accepted")
}
if err := ensurePrivateDir(filepath.Join(base, "fresh"), true); err != nil {
t.Fatal(err)
}
}
internal/scan/scan.go
+12 -6
View File
@@ -176,7 +176,13 @@ func Start(ctx context.Context, root string, opts Options) (*Scanner, error) {
}
t.Stats.Dirs = 1
c := &controller{s: s, t: t, opts: opts, links: map[inodeKey]struct{}{}}
if opts.Follow != FollowNone {
// Loop detection needs inode identity; without it (Windows and other
// portable-fallback platforms) symlinks are not followed at all.
if opts.Follow != FollowNone && !m.HasIno {
c.opts.Follow = FollowNone
t.Stats.Follow = "none (no inode identity on this platform)"
}
if c.opts.Follow != FollowNone {
c.visited = map[inodeKey]struct{}{{m.Dev, m.Ino}: {}}
}
c.rootDev = m.Dev
@@ -271,7 +277,7 @@ func (c *controller) run(ctx context.Context) {
}
c.t.Lock()
for i := range batch {
if c.apply(&batch[i], cancelled) {
if c.apply(&batch[i]) {
inflight--
}
batch[i] = result{}
@@ -292,7 +298,7 @@ func (c *controller) run(ctx context.Context) {
st := &c.t.Stats
if cancelled {
st.Cancelled = true
st.Unscanned = int64(len(c.pending) - c.head)
st.Unscanned += int64(len(c.pending) - c.head)
for _, p := range c.pending[c.head:] {
c.t.Node(p.id).Flags |= inventory.FlagIncomplete
}
@@ -352,7 +358,7 @@ func setMeta(n *inventory.Node, m platform.Meta) {
// apply merges one result into the tree. It reports whether the result
// completed its directory. Called with the tree write-locked.
func (c *controller) apply(r *result, cancelled bool) bool {
func (c *controller) apply(r *result) bool {
t := c.t
j := r.job
dir := j.id
@@ -366,7 +372,7 @@ func (c *controller) apply(r *result, cancelled bool) bool {
switch {
case r.err == nil:
dn.Flags |= inventory.FlagScanned
case errors.Is(r.err, context.Canceled) || errors.Is(r.err, context.DeadlineExceeded) || cancelled:
case errors.Is(r.err, context.Canceled) || errors.Is(r.err, context.DeadlineExceeded):
dn.Flags |= inventory.FlagIncomplete
t.Stats.Unscanned++
default:
@@ -447,7 +453,7 @@ func (c *controller) addEntry(j *job, m platform.Meta, d *inventory.Delta) {
case err != nil:
st.BrokenLinks++
broken = true
case tm.Mode.IsDir() && (c.opts.Follow == FollowAll || c.opts.Follow == FollowSameFS && tm.Dev == j.dev):
case tm.Mode.IsDir() && tm.HasIno && (c.opts.Follow == FollowAll || c.opts.Follow == FollowSameFS && tm.Dev == j.dev):
target, followed = tm, true
kind = inventory.KindDir
}
internal/snapshot/hostile_test.go
+131
View File
@@ -0,0 +1,131 @@
package snapshot
import (
"bytes"
"compress/gzip"
"crypto/sha256"
"encoding/binary"
"io"
"strings"
"testing"
"github.com/andydixon/mapsize/internal/brand"
"github.com/andydixon/mapsize/internal/export"
"github.com/andydixon/mapsize/internal/inventory"
)
// craft builds a well-formed (correctly checksummed) but hostile snapshot.
func craft(n int, meta string, rec func(i int, w *writer)) []byte {
var out bytes.Buffer
out.WriteString(brand.SnapshotMagic)
out.Write(binary.LittleEndian.AppendUint16(nil, Version))
out.Write(binary.LittleEndian.AppendUint16(nil, flagGzipped))
gz, _ := gzip.NewWriterLevel(&out, gzip.BestCompression)
h := sha256.New()
w := &writer{w: io.MultiWriter(gz, h)}
w.uvarint(uint64(len(meta)))
w.bytes([]byte(meta))
w.uvarint(1)
w.str("")
w.uvarint(uint64(n))
for i := range n {
rec(i, w)
}
w.uvarint(0)
gz.Close()
out.Write(h.Sum(nil))
return out.Bytes()
}
func node(w *writer, pd uint64, kind inventory.Kind, name string, size int64) {
w.uvarint(pd)
w.bytes([]byte{byte(kind), 0})
w.uvarint(0)
w.str(name)
w.varint(size)
w.varint(size)
w.varint(0)
for range 5 {
w.uvarint(0)
}
}
func chain(n int) []byte {
return craft(n, `{"root":"/r"}`, func(i int, w *writer) {
if i == 0 {
node(w, 0, inventory.KindDir, "", 0)
} else {
node(w, 1, inventory.KindDir, "a", 0)
}
})
}
func TestDeepChainRejected(t *testing.T) {
if _, err := Load(bytes.NewReader(chain(MaxDepth + 1))); err != nil {
t.Fatalf("depth at the limit must load: %v", err)
}
_, err := Load(bytes.NewReader(chain(MaxDepth + 2)))
if err == nil || !strings.Contains(err.Error(), "deeper") {
t.Fatalf("over-deep chain accepted: %v", err)
}
}
func TestDecompressionBombLimited(t *testing.T) {
flat := craft(100_000, `{"root":"/r"}`, func(i int, w *writer) {
if i == 0 {
node(w, 0, inventory.KindDir, "", 0)
} else {
node(w, uint64(i), inventory.KindFile, "f", 0)
}
})
_, err := LoadLimit(bytes.NewReader(flat), 64<<10)
if err == nil || !strings.Contains(err.Error(), "limit") {
t.Fatalf("bomb not limited: %v", err)
}
if got := bodyLimit(int64(len(flat))); got < 64<<20 {
t.Fatalf("bodyLimit floor: %d", got)
}
}
func TestHugeSizesDoNotWrap(t *testing.T) {
data := craft(3, `{"root":"/r"}`, func(i int, w *writer) {
if i == 0 {
node(w, 0, inventory.KindDir, "", 0)
} else {
node(w, uint64(i), inventory.KindFile, "f", MaxSize)
}
})
tr, err := Load(bytes.NewReader(data))
if err != nil {
t.Fatal(err)
}
if tr.Node(0).TotSize < 0 {
t.Fatal("aggregate wrapped negative")
}
over := craft(2, `{"root":"/r"}`, func(i int, w *writer) {
if i == 0 {
node(w, 0, inventory.KindDir, "", 0)
} else {
node(w, 1, inventory.KindFile, "f", MaxSize+1)
}
})
if _, err := Load(bytes.NewReader(over)); err == nil {
t.Fatal("size above MaxSize accepted")
}
}
func TestHostileMetadataSanitisedInSummary(t *testing.T) {
data := craft(1, `{"root":"/r\u001b[2J","excludes":["\u001b]0;PWNED\u0007"],"excluded":1}`, func(i int, w *writer) {
node(w, 0, inventory.KindDir, "", 0)
})
tr, err := Load(bytes.NewReader(data))
if err != nil {
t.Fatal(err)
}
tr.Stats.Excluded = 1
var b bytes.Buffer
export.Summary(&b, tr)
if strings.ContainsAny(b.String(), "\x1b\x07") {
t.Fatalf("raw control characters in summary: %q", b.String())
}
}
internal/snapshot/snapshot.go
+55 -13
View File
@@ -47,13 +47,18 @@ const Version = 1
// Limits applied when reading untrusted snapshots.
const (
MaxNodes = 1<<31 - 1
MaxNameLen = 1 << 16
MaxMetaLen = 16 << 20
MaxExtLen = 64
MaxMsgLen = 4096
maxExts = 1<<16 - 1
flagGzipped = 1
MaxNodes = 1<<31 - 1
MaxDepth = 4096 // far beyond any real path; bounds recursion in exporters
MaxSize = 1 << 60 // per-node size cap (1 EiB); keeps aggregates from overflowing
// DefaultMaxBody caps the decompressed body for Load. LoadFile uses a
// limit proportional to the file size instead (see bodyLimit).
DefaultMaxBody = 4 << 30
MaxNameLen = 1 << 16
MaxMetaLen = 16 << 20
MaxExtLen = 64
MaxMsgLen = 4096
maxExts = 1<<16 - 1
flagGzipped = 1
)
type meta struct {
@@ -305,8 +310,35 @@ func (r *reader) str(max uint64, what string) string {
return string(r.bytesN(r.uvarint(max, what+" length")))
}
// Load reads and validates a snapshot.
func Load(in io.Reader) (*inventory.Tree, error) {
// limitedReader fails (rather than silently truncating) past n bytes, so a
// decompression bomb is reported as such.
type limitedReader struct {
r io.Reader
n int64
}
func (l *limitedReader) Read(p []byte) (int, error) {
if l.n <= 0 {
return 0, errors.New("snapshot: decompressed size exceeds limit (possible decompression bomb)")
}
if int64(len(p)) > l.n {
p = p[:l.n]
}
n, err := l.r.Read(p)
l.n -= int64(n)
return n, err
}
// bodyLimit allows decompressed bodies up to 64× the compressed file size
// (real snapshots compress ~6×), with a 64 MiB floor for tiny files.
func bodyLimit(fileSize int64) int64 { return max(64<<20, fileSize*64) }
// Load reads and validates a snapshot, allowing up to DefaultMaxBody bytes
// of decompressed data.
func Load(in io.Reader) (*inventory.Tree, error) { return LoadLimit(in, DefaultMaxBody) }
// LoadLimit is Load with an explicit decompressed-size limit.
func LoadLimit(in io.Reader, maxBody int64) (*inventory.Tree, error) {
br := bufio.NewReaderSize(in, 1<<20)
hdr := make([]byte, len(brand.SnapshotMagic)+4)
if _, err := io.ReadFull(br, hdr); err != nil {
@@ -333,7 +365,7 @@ func Load(in io.Reader) (*inventory.Tree, error) {
pr, pw := io.Pipe()
defer pr.Close() // unblocks the copier if we bail out early
go func() {
_, err := io.CopyBuffer(pw, io.TeeReader(gz, h), make([]byte, 256<<10))
_, err := io.CopyBuffer(pw, io.TeeReader(&limitedReader{gz, maxBody}, h), make([]byte, 256<<10))
pw.CloseWithError(err)
}()
r := &reader{r: bufio.NewReaderSize(pr, 1<<16)}
@@ -358,6 +390,7 @@ func Load(in io.Reader) (*inventory.Tree, error) {
}
extIndex := t.ExtIndexer()
nNodes := r.uvarint(MaxNodes, "node count")
depth := []uint16{0} // per node, for the depth limit; parents precede children
if nNodes == 0 {
return nil, errors.New("snapshot: no root node")
}
@@ -380,8 +413,8 @@ func Load(in io.Reader) (*inventory.Tree, error) {
r.fail("node %d: invalid kind/category", i)
break
}
if size < 0 || alloc < 0 {
r.fail("node %d: negative size", i)
if size < 0 || alloc < 0 || size > MaxSize || alloc > MaxSize {
r.fail("node %d: size out of range", i)
break
}
var id inventory.NodeID
@@ -407,7 +440,12 @@ func Load(in io.Reader) (*inventory.Tree, error) {
r.fail("node %d: invalid name %q", i, name)
break
}
if d := depth[parent] + 1; d > MaxDepth {
r.fail("node %d: deeper than %d levels", i, MaxDepth)
break
}
id = t.Add(parent, name, kind)
depth = append(depth, depth[parent]+1)
}
n := t.Node(id)
n.Cat, n.Flags = cat, flags
@@ -469,5 +507,9 @@ func LoadFile(path string) (*inventory.Tree, error) {
return nil, err
}
defer f.Close()
return Load(f)
limit := int64(DefaultMaxBody)
if fi, err := f.Stat(); err == nil {
limit = bodyLimit(fi.Size())
}
return LoadLimit(f, limit)
}
internal/tui/actions.go
+12 -5
View File
@@ -17,8 +17,9 @@ import (
// ---- Trash ---------------------------------------------------------------
type trashDoneMsg struct {
id inventory.NodeID
err error
tree *inventory.Tree // the tree id belongs to; results for a replaced tree are dropped
id inventory.NodeID
err error
}
// requestTrash asks for confirmation before moving the selection to the
@@ -49,7 +50,8 @@ func (m *Model) requestTrash() tea.Cmd {
"", "It will be moved to the " + platform.TrashName + ", not permanently deleted."},
yes: "move to " + platform.TrashName,
onYes: func(m *Model) tea.Cmd {
return func() tea.Msg { return trashDoneMsg{id, platform.Trash(path)} }
t := m.tree
return func() tea.Msg { return trashDoneMsg{t, id, platform.Trash(path)} }
},
})
return nil
@@ -59,6 +61,10 @@ func (m *Model) trashDone(msg trashDoneMsg) tea.Cmd {
if msg.err != nil {
return m.warn("Trash failed: " + textutil.Sanitize(msg.err.Error()))
}
if msg.tree != m.tree || m.scanning {
// A rescan replaced the inventory; the new scan reflects the change.
return m.info("Moved to " + platform.TrashName)
}
// The scan has finished, so the UI goroutine is the tree's only writer.
t := m.tree
t.Lock()
@@ -96,6 +102,7 @@ type dupState struct {
type (
dupProgressMsg struct{}
dupDoneMsg struct {
finder *duplicate.Finder // identifies the search; stale results are dropped
groups []duplicate.Group
err error
}
@@ -128,7 +135,7 @@ func (m *Model) startDuplicates() tea.Cmd {
t := m.tree
return tea.Batch(m.startTicking(), func() tea.Msg {
g, err := f.Find(ctx, t, duplicate.Options{MinSize: 1, Workers: 4})
return dupDoneMsg{g, err}
return dupDoneMsg{f, g, err}
})
},
})
@@ -147,7 +154,7 @@ func (m *Model) ensureDupView() {
}
func (m *Model) dupMsg(msg tea.Msg) tea.Cmd {
if d, ok := msg.(dupDoneMsg); ok && m.dups != nil {
if d, ok := msg.(dupDoneMsg); ok && m.dups != nil && d.finder == m.dups.finder {
m.dups.running = false
m.dups.groups, m.dups.err = d.groups, d.err
m.dirty = true
internal/tui/model.go
+14 -2
View File
@@ -378,10 +378,22 @@ func (m *Model) rescan() tea.Cmd {
m.scanner, m.tree, m.cancel, m.scanning, m.ctrlC = s, t, cancel, s != nil, false
m.zoom, m.sel = t.Root(), noSel
m.kidsCache = map[inventory.NodeID]*kids{}
m.dups = nil
if m.dups != nil {
m.dups.cancel()
m.dups = nil
// Drop the duplicates view: its rows referred to the old tree.
for i, v := range m.views {
if _, ok := v.(*dupView); ok {
m.views = append(m.views[:i], m.views[i+1:]...)
m.view = 0
break
}
}
}
m.search.result = nil
m.search.gen++ // results computed against the old tree are discarded
m.invalidate()
return tea.Batch(waitScan(s), m.startTicking(), m.info("Rescanning…"))
return tea.Batch(waitScan(s), m.startTicking(), m.info("Rescanning…"), m.applyFilter())
}
// resolvePendingZoom re-zooms into the remembered directory once the new