diff options
Diffstat (limited to 'internal/admin')
| -rw-r--r-- | internal/admin/api_files.go | 28 | ||||
| -rw-r--r-- | internal/admin/api_files_path_test.go | 66 |
2 files changed, 89 insertions, 5 deletions
diff --git a/internal/admin/api_files.go b/internal/admin/api_files.go index 496b08d..f2ef928 100644 --- a/internal/admin/api_files.go +++ b/internal/admin/api_files.go @@ -1,6 +1,7 @@ package admin import ( + "fmt" "io" "net/http" "os" @@ -8,6 +9,17 @@ import ( "strings" ) +// resolveDataPath resolves a request-supplied path inside the admin data +// directory, rejecting paths that would escape it via .. segments. The +// returned path is always filepath.Join(dataDir, cleaned relative path). +func resolveDataPath(dataDir, relPath string) (string, error) { + clean := filepath.Clean(strings.TrimPrefix(strings.TrimPrefix(relPath, "/"), "data/")) + if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path escapes the data directory: %s", relPath) + } + return filepath.Join(dataDir, clean), nil +} + func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { http.Error(w, "method not allowed", http.StatusMethodNotAllowed) @@ -36,7 +48,11 @@ func (s *AdminServer) handleFileRename(w http.ResponseWriter, r *http.Request) { relPath := strings.TrimPrefix(body.Path, "/") relPath = strings.TrimPrefix(relPath, "data/") - oldAbs := filepath.Join(s.dataDir, relPath) + oldAbs, err := resolveDataPath(s.dataDir, relPath) + if err != nil { + writeJSON(w, map[string]any{"error": err.Error()}) + return + } oldContent, err := snapshotFile(oldAbs) if err != nil { @@ -77,11 +93,16 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { relPath := r.URL.Query().Get("path") relPath = strings.TrimPrefix(relPath, "/") relPath = strings.TrimPrefix(relPath, "data/") + fullPath, pathErr := resolveDataPath(s.dataDir, relPath) + if pathErr != nil { + writeJSON(w, map[string]any{"error": pathErr.Error()}) + return + } switch r.Method { case http.MethodGet: if relPath == "" { - entries, err := os.ReadDir(s.dataDir) + entries, err := os.ReadDir(fullPath) if err != nil { writeJSON(w, map[string]any{"error": err.Error()}) return @@ -137,8 +158,6 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { writeJSON(w, map[string]any{"error": "missing path"}) return } - fullPath := filepath.Join(s.dataDir, relPath) - body, err := io.ReadAll(r.Body) if err != nil { writeJSON(w, map[string]any{"error": "read body: " + err.Error()}) @@ -180,7 +199,6 @@ func (s *AdminServer) handleFiles(w http.ResponseWriter, r *http.Request) { writeJSON(w, map[string]any{"error": "missing path"}) return } - fullPath := filepath.Join(s.dataDir, relPath) oldContent, _ := snapshotFile(fullPath) if err := os.Remove(fullPath); err != nil { writeJSON(w, map[string]any{"error": "delete error: " + err.Error()}) diff --git a/internal/admin/api_files_path_test.go b/internal/admin/api_files_path_test.go new file mode 100644 index 0000000..6d660fb --- /dev/null +++ b/internal/admin/api_files_path_test.go @@ -0,0 +1,66 @@ +package admin + +import ( + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestFilesStayInsideDataDir: the raw file editor must reject request paths +// that escape the data directory via .. segments — writes, reads, and +// deletes alike. +func TestFilesStayInsideDataDir(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "data") + if err := os.Mkdir(dir, 0700); err != nil { + t.Fatal(err) + } + target := filepath.Join(root, "outside.txt") + if err := os.WriteFile(target, []byte("original"), 0600); err != nil { + t.Fatal(err) + } + s := &AdminServer{dataDir: dir, undoStack: NewUndoStack(dir)} + + // Write traversal. + r := httptest.NewRequest("PUT", "/api/files?path=../outside.txt", strings.NewReader("overwritten")) + w := httptest.NewRecorder() + s.handleFiles(w, r) + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "original" { + t.Fatalf("PUT traversal overwrote outside the data dir: content=%q response=%s", got, w.Body.String()) + } + + // Read traversal must not reveal the file either. + r = httptest.NewRequest("GET", "/api/files?path=../outside.txt", nil) + w = httptest.NewRecorder() + s.handleFiles(w, r) + if strings.Contains(w.Body.String(), "original") { + t.Fatalf("GET traversal read outside the data dir: %s", w.Body.String()) + } + + // Deep traversal segments are rejected too. + r = httptest.NewRequest("DELETE", "/api/files?path=rooms/../../outside.txt", nil) + w = httptest.NewRecorder() + s.handleFiles(w, r) + if got, err := os.ReadFile(target); err != nil || string(got) != "original" { + t.Fatalf("DELETE traversal removed the outside file: %q %v", got, err) + } + + // Renaming into a traversal path is rejected as well. + if err := os.WriteFile(filepath.Join(dir, "inside.txt"), []byte("x"), 0600); err != nil { + t.Fatal(err) + } + r = httptest.NewRequest("POST", "/api/files/rename", strings.NewReader(`{"path":"inside.txt","new_name":"../escaped.txt"}`)) + // The rename handler validates the new name itself; ../ in a name is + // rejected by the slash check, so this only guards the source path. + w = httptest.NewRecorder() + s.handleFileRename(w, r) + if _, err := os.Stat(filepath.Join(root, "escaped.txt")); err == nil { + t.Fatal("rename escaped the data directory") + } +} |
