diff options
Diffstat (limited to 'internal/notify')
| -rw-r--r-- | internal/notify/service.go | 7 | ||||
| -rw-r--r-- | internal/notify/service_test.go | 34 | ||||
| -rw-r--r-- | internal/notify/store_test.go | 12 |
3 files changed, 49 insertions, 4 deletions
diff --git a/internal/notify/service.go b/internal/notify/service.go index d46293f..2442127 100644 --- a/internal/notify/service.go +++ b/internal/notify/service.go @@ -14,7 +14,7 @@ package notify import ( "log" "os" - "strings" + "path/filepath" "sync" "time" @@ -52,12 +52,13 @@ func NewService(conn *dbus.Conn, dir string) *Service { } // removeImage unlinks only what the daemon wrote, so a client's own image-path -// is never touched. +// is never touched. A cleaned path must sit directly inside the image +// directory, which also refuses a `..` traversal to a sibling daemon file. func (s *Service) removeImage(path string) { if path == "" { return } - if !strings.HasPrefix(path, ImagesDir(s.dir)+string(os.PathSeparator)) { + if filepath.Dir(filepath.Clean(path)) != ImagesDir(s.dir) { return } os.Remove(path) diff --git a/internal/notify/service_test.go b/internal/notify/service_test.go index fb47b98..871a0c7 100644 --- a/internal/notify/service_test.go +++ b/internal/notify/service_test.go @@ -144,6 +144,40 @@ func TestCloseNotificationEmitsReasonThree(t *testing.T) { } } +func TestRemoveImageRefusesPathsOutsideTheImageDir(t *testing.T) { + dir := t.TempDir() + imgDir := ImagesDir(dir) + if err := os.MkdirAll(imgDir, 0o700); err != nil { + t.Fatal(err) + } + sibling := filepath.Join(dir, "queue.json") + if err := os.WriteFile(sibling, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + owned := filepath.Join(imgDir, "1.png") + if err := os.WriteFile(owned, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + s := &Service{dir: dir} + + s.removeImage(filepath.Join(imgDir, "..", "queue.json")) + if _, err := os.Stat(sibling); err != nil { + t.Fatalf("traversal removed a sibling: %v", err) + } + s.removeImage(sibling) + if _, err := os.Stat(sibling); err != nil { + t.Fatalf("unrelated path removed a sibling: %v", err) + } + s.removeImage("") + if _, err := os.Stat(sibling); err != nil { + t.Fatalf("empty path removed a sibling: %v", err) + } + s.removeImage(owned) + if _, err := os.Stat(owned); !os.IsNotExist(err) { + t.Fatalf("owned image not removed: %v", err) + } +} + // A natural expiry must drop the timer entry, or the map grows by one timer per // id for the whole session. No bus is needed: the store's emit is a no-op here, // so the test exercises arm's closure directly. diff --git a/internal/notify/store_test.go b/internal/notify/store_test.go index a0ab7be..13f78dc 100644 --- a/internal/notify/store_test.go +++ b/internal/notify/store_test.go @@ -133,7 +133,7 @@ func TestHistoryRingCapsAtTwenty(t *testing.T) { } } -func TestStoreRemovesImageOnDismissAndExpire(t *testing.T) { +func TestStoreRemovesImageOnDismiss(t *testing.T) { var removed []string s := NewStore(func(uint32, uint32) {}, func(_, _ []Popup) {}, func(p string) { removed = append(removed, p) }) id, _ := s.Add(&Popup{Image: "/run/img/1.png"}, "", 0) @@ -144,6 +144,16 @@ func TestStoreRemovesImageOnDismissAndExpire(t *testing.T) { } } +func TestStoreRemovesImageOnExpire(t *testing.T) { + var removed []string + s := NewStore(func(uint32, uint32) {}, func(_, _ []Popup) {}, func(p string) { removed = append(removed, p) }) + id, _ := s.Add(&Popup{Image: "/run/img/expire.png"}, "", 0) + s.Expire(id) + if len(removed) != 1 || removed[0] != "/run/img/expire.png" { + t.Fatalf("expire removed %v", removed) + } +} + func TestStoreRemovesImageOnReplace(t *testing.T) { var removed []string s := NewStore(func(uint32, uint32) {}, func(_, _ []Popup) {}, func(p string) { removed = append(removed, p) }) |
