plan 8: trash refuses unsafe entry names and relative paths

3 files changed, 84 insertions, 4 deletions

diff --git a/Makefile b/Makefile
index 23e16ed..3cfc8f1 100644
--- a/Makefile
+++ b/Makefile
@@ -85,7 +85,9 @@ FUZZ_TARGETS = \
 	internal/sexp:FuzzParse \
 	internal/journal:FuzzEscapeRoundTrip \
 	internal/journal:FuzzParseLine \
-	internal/journal:FuzzEntryRoundTrip
+	internal/journal:FuzzEntryRoundTrip \
+	internal/trash:FuzzPercentRoundTrip \
+	internal/trash:FuzzParsePath
 
 fuzz: ## run every fuzz target for FUZZTIME each (default 20s); a crasher is saved under testdata/fuzz
 	@for t in $(FUZZ_TARGETS); do \
diff --git a/internal/trash/fuzz_test.go b/internal/trash/fuzz_test.go
new file mode 100644
index 0000000..ffda6d3
--- /dev/null
+++ b/internal/trash/fuzz_test.go
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package trash
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// FuzzPercentRoundTrip: percentEncode leaves only unreserved characters,
+// '/' and %XX escapes, and percentDecode gives back exactly the path
+// encoded - a newline or invalid byte in a name included.
+func FuzzPercentRoundTrip(f *testing.F) {
+	for _, s := range []string{"/home/x/a b.pdf", "/dl/zażółć.pdf", "/dl/%41", "/dl/new\nline", "/dl/\xff", "%", "%%4", "/dl/[Trash Info]"} {
+		f.Add(s)
+	}
+	f.Fuzz(func(t *testing.T, s string) {
+		e := percentEncode(s)
+		for i := 0; i < len(e); i++ {
+			if c := e[i]; !isUnreserved(c) && c != '/' && c != '%' {
+				t.Fatalf("percentEncode(%q) = %q holds %q", s, e, c)
+			}
+		}
+		if got := percentDecode(e); got != s {
+			t.Fatalf("percentDecode(percentEncode(%q)) = %q", s, got)
+		}
+	})
+}
+
+// FuzzParsePath: a trashinfo of any content gives an absolute path or an
+// error - never a panic, and never a relative path that Restore would
+// resolve against the working directory.
+func FuzzParsePath(f *testing.F) {
+	f.Add("[Trash Info]\nPath=/home/x/a.pdf\nDeletionDate=2026-09-14T10:00:00\n")
+	f.Add("[Trash Info]\nPath=relative/a.pdf\n")
+	f.Add("Path=")
+	f.Add("Path=%2F..%2Fetc")
+	f.Fuzz(func(t *testing.T, info string) {
+		p, err := parsePath(info)
+		if err == nil && !filepath.IsAbs(p) {
+			t.Fatalf("parsePath(%q) = %q, which is not absolute", info, p)
+		}
+	})
+}
+
+// TestRestoreRefusesUnsafeEntry: an entry name that is not a plain name
+// inside the Trash - as a damaged or hand-edited log could hold - is
+// refused before anything is read or moved. "../../outside" would
+// otherwise reach a trashinfo and a file beside the Trash directory.
+func TestRestoreRefusesUnsafeEntry(t *testing.T) {
+	h := sandbox(t)
+	share := filepath.Join(h, "share")
+	write(t, filepath.Join(share, "outside.trashinfo"), "[Trash Info]\nPath="+filepath.Join(h, "restored.txt")+"\n")
+	write(t, filepath.Join(share, "outside"), "not in the trash")
+	for _, entry := range []string{"", ".", "..", "../../outside", "a/b"} {
+		if _, err := Restore(entry); err == nil || !strings.Contains(err.Error(), "bad entry name") {
+			t.Errorf("Restore(%q) = %v, want a bad entry name error", entry, err)
+		}
+	}
+	if _, err := os.Stat(filepath.Join(h, "restored.txt")); !os.IsNotExist(err) {
+		t.Errorf("a file outside the Trash was restored: %v", err)
+	}
+}
diff --git a/internal/trash/trash.go b/internal/trash/trash.go
index cc9bc23..b96c3aa 100644
--- a/internal/trash/trash.go
+++ b/internal/trash/trash.go
@@ -174,7 +174,9 @@ func percentDecode(s string) string {
 }
 
 // Restore moves an entry back to the Path recorded in its .trashinfo and
-// removes the .trashinfo. It refuses when that path already exists.
+// removes the .trashinfo. It refuses when that path already exists. It also
+// refuses an entry that is not a plain name inside the Trash, and a
+// trashinfo whose Path is not absolute.
 //
 // Once the rename back to the original path has succeeded, removing the
 // .trashinfo is best-effort: that file back in place is the substantive
@@ -183,6 +185,11 @@ func percentDecode(s string) string {
 // as an error — Restore returns (path, nil) regardless — and the
 // .trashinfo may survive as a stale, otherwise-harmless record.
 func Restore(entry string) (restored string, err error) {
+	// An entry comes from the log; it must name something inside the Trash
+	// (spec §15.1), never a path that climbs out of it.
+	if entry == "" || entry == "." || entry == ".." || strings.ContainsRune(entry, '/') {
+		return "", fmt.Errorf("trash: bad entry name %q", entry)
+	}
 	infoPath := filepath.Join(infoDir(), entry+".trashinfo")
 	b, err := os.ReadFile(infoPath)
 	if err != nil {
@@ -211,11 +218,17 @@ func Restore(entry string) (restored string, err error) {
 	return path, nil
 }
 
-// parsePath extracts and decodes the Path= line of a .trashinfo file.
+// parsePath extracts and decodes the Path= line of a .trashinfo file. The
+// path must be absolute: Restore must never resolve one against the
+// working directory.
 func parsePath(info string) (string, error) {
 	for _, line := range strings.Split(info, "\n") {
 		if v, ok := strings.CutPrefix(line, "Path="); ok {
-			return percentDecode(v), nil
+			p := percentDecode(v)
+			if !filepath.IsAbs(p) {
+				return "", fmt.Errorf("trashinfo path %q is not absolute", p)
+			}
+			return p, nil
 		}
 	}
 	return "", errors.New("trashinfo has no path")