prevent path traversal for public items

ybizeul · Aug 8, 2024 · f8b69b5 · f8b69b5
1 parent a72b155
commit f8b69b5
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 5 deletions.
diff --git a/internal/feed/feed.go b/internal/feed/feed.go
@@ -250,7 +250,7 @@ func (feed *Feed) GetPublicItem(i string) (*PublicFeedItem, error) {
 		return nil, FeedErrorInvalidFeedItem
 	}
 
-	s, err := os.Stat(path.Join(feed.Path, i))
+	s, err := os.Stat(path.Join(feed.Path, path.Join("/", i)))
 
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -409,7 +409,7 @@ func (f *Feed) AddItem(contentType string, r io.Reader) error {
 func (f *Feed) RemoveItem(item string) error {
 	fL.Logger.Debug("Remove Item", slog.String("name", item), slog.String("feed", f.Path))
 
-	itemPath := path.Join(f.Path, item)
+	itemPath := path.Join(f.Path, path.Join("/", item))
 
 	// Save public item before deletion for notification later
 	publicItem, err := f.GetPublicItem(item)
@@ -427,8 +427,10 @@ func (f *Feed) RemoveItem(item string) error {
 	}
 
 	// Notify all connected websockets
-	if err = f.WebSocketManager.NotifyRemove(publicItem); err != nil {
-		return err
+	if f.WebSocketManager != nil {
+		if err = f.WebSocketManager.NotifyRemove(publicItem); err != nil {
+			return err
+		}
 	}
 
 	fL.Logger.Debug("Removed Item", slog.String("name", item), slog.String("feed", f.Path))

diff --git a/internal/feed/feed_test.go b/internal/feed/feed_test.go
@@ -33,7 +33,7 @@ func TestGetFeedItemData(t *testing.T) {
 	}
 }
 
-func TestPathTraversal(t *testing.T) {
+func TestPathTraversalGet(t *testing.T) {
 	t.Cleanup(func() {
 		os.RemoveAll("tests/feed1")
 		os.RemoveAll("tests/feed2")
@@ -54,3 +54,76 @@ func TestPathTraversal(t *testing.T) {
 		t.Fatal("Path traversal not blocked")
 	}
 }
+
+func TestPathTraversalDelete(t *testing.T) {
+	t.Cleanup(func() {
+		os.RemoveAll("tests/feed1")
+		os.RemoveAll("tests/feed2")
+	})
+	_, err := NewFeed("tests/feed1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := NewFeed("tests/feed2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = f.RemoveItem("../feed1/config.json")
+
+	if err == nil {
+		t.Fatal("Path traversal not blocked")
+	}
+}
+
+func TestPathTraversalPublicItem(t *testing.T) {
+	t.Cleanup(func() {
+		os.RemoveAll("tests/feed1")
+		os.RemoveAll("tests/feed2")
+	})
+	_, err := NewFeed("tests/feed1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := NewFeed("tests/feed2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	p, err := f.GetPublicItem("../feed1/config.json")
+
+	if p != nil || err == nil {
+		t.Fatal("Path traversal not blocked")
+	}
+}
+
+func TestPublicItem(t *testing.T) {
+	t.Cleanup(func() {
+		os.RemoveAll("tests/feed1")
+	})
+	f, err := NewFeed("tests/feed1")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	reader := bytes.NewReader([]byte("test"))
+
+	err = f.AddItem("text/plain", reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	pf, err := f.Public()
+	if err != nil {
+		t.Fatal(err)
+	}
+	i := pf.Items[0]
+
+	p, err := f.GetPublicItem(i.Name)
+
+	if p == nil || err != nil {
+		t.Fatal(err)
+	}
+}