diff options
| -rw-r--r-- | Makefile | 4 | ||||
| -rw-r--r-- | internal/trash/fuzz_test.go | 65 | ||||
| -rw-r--r-- | internal/trash/trash.go | 19 |
3 files changed, 84 insertions, 4 deletions
@@ -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") |