From 989f87fe086b88830580847803f875aa2c9776bb Mon Sep 17 00:00:00 2001 From: shaw Date: Thu, 7 May 2026 09:46:45 +0800 Subject: [PATCH] fix: harden markdown page image paths --- backend/internal/handler/page_handler.go | 84 +++++++++++++-- backend/internal/handler/page_handler_test.go | 102 ++++++++++++++++++ .../service/content_moderation_test.go | 8 ++ frontend/src/views/user/CustomPageView.vue | 27 ++++- 4 files changed, 211 insertions(+), 10 deletions(-) create mode 100644 backend/internal/handler/page_handler_test.go diff --git a/backend/internal/handler/page_handler.go b/backend/internal/handler/page_handler.go index a3e4f5d2..7d4d5078 100644 --- a/backend/internal/handler/page_handler.go +++ b/backend/internal/handler/page_handler.go @@ -3,6 +3,7 @@ package handler import ( "encoding/json" "net/http" + "net/url" "os" "path/filepath" "regexp" @@ -111,15 +112,9 @@ func (h *PageHandler) ServePageImage(c *gin.Context) { return } - if filename == "" || strings.Contains(filename, "..") || strings.Contains(filename, "/") || strings.Contains(filename, "\\") { - c.Status(http.StatusNotFound) - return - } - imagesDir := filepath.Join(h.pagesDir, slug) - filePath := filepath.Join(imagesDir, filename) - cleaned := filepath.Clean(filePath) - if !strings.HasPrefix(cleaned, filepath.Clean(imagesDir)) { + cleaned, ok := resolvePageImagePath(h.pagesDir, imagesDir, filename) + if !ok { c.Status(http.StatusNotFound) return } @@ -133,6 +128,79 @@ func (h *PageHandler) ServePageImage(c *gin.Context) { c.File(cleaned) } +func resolvePageImagePath(pagesDir, imagesDir, filename string) (string, bool) { + relPath, ok := cleanPageImageRelativePath(filename) + if !ok { + return "", false + } + + cleanedPagesDir := filepath.Clean(pagesDir) + cleanedImagesDir := filepath.Clean(imagesDir) + cleanedTarget := filepath.Clean(filepath.Join(cleanedImagesDir, relPath)) + if !isPathWithinBase(cleanedTarget, cleanedImagesDir) { + return "", false + } + + realPagesDir, err := filepath.EvalSymlinks(cleanedPagesDir) + if err != nil { + return "", false + } + realImagesDir, err := filepath.EvalSymlinks(cleanedImagesDir) + if err != nil || !isPathWithinBase(realImagesDir, realPagesDir) { + return "", false + } + realTarget, err := filepath.EvalSymlinks(cleanedTarget) + if err != nil || !isPathWithinBase(realTarget, realImagesDir) { + return "", false + } + return realTarget, true +} + +func cleanPageImageRelativePath(filename string) (string, bool) { + if filename == "" { + return "", false + } + if strings.HasPrefix(filename, "/") { + return "", false + } + decoded, err := url.PathUnescape(filename) + if err != nil { + return "", false + } + if decoded == "" || strings.HasPrefix(decoded, "/") || strings.Contains(decoded, "\\") || strings.ContainsRune(decoded, 0) { + return "", false + } + + parts := make([]string, 0) + for _, part := range strings.Split(decoded, "/") { + switch part { + case "", ".": + continue + case "..": + return "", false + default: + parts = append(parts, part) + } + } + if len(parts) == 0 { + return "", false + } + + relPath := filepath.Join(parts...) + if filepath.IsAbs(relPath) || filepath.VolumeName(relPath) != "" { + return "", false + } + return relPath, true +} + +func isPathWithinBase(path, base string) bool { + rel, err := filepath.Rel(filepath.Clean(base), filepath.Clean(path)) + if err != nil { + return false + } + return rel != "." && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + // findSlugVisibility looks up the slug in custom_menu_items and returns (visibility, found). func (h *PageHandler) findSlugVisibility(c *gin.Context, slug string) (string, bool) { if h.settingService == nil { diff --git a/backend/internal/handler/page_handler_test.go b/backend/internal/handler/page_handler_test.go new file mode 100644 index 00000000..0a9f0d96 --- /dev/null +++ b/backend/internal/handler/page_handler_test.go @@ -0,0 +1,102 @@ +package handler + +import ( + "os" + "path/filepath" + "testing" +) + +func TestCleanPageImageRelativePath(t *testing.T) { + tests := []struct { + name string + in string + want string + ok bool + }{ + {name: "single filename", in: "logo.png", want: "logo.png", ok: true}, + {name: "nested path", in: "images/logo.png", want: filepath.Join("images", "logo.png"), ok: true}, + {name: "dot prefix", in: "./logo.png", want: "logo.png", ok: true}, + {name: "url escaped slash", in: "images%2Flogo.png", want: filepath.Join("images", "logo.png"), ok: true}, + {name: "parent traversal", in: "../secret.png", ok: false}, + {name: "encoded parent traversal", in: "%2e%2e/secret.png", ok: false}, + {name: "backslash traversal", in: `images\secret.png`, ok: false}, + {name: "absolute path", in: "/etc/passwd", ok: false}, + {name: "encoded absolute path", in: "%2fetc/passwd", ok: false}, + {name: "encoded nul byte", in: "logo.png%00", ok: false}, + {name: "invalid escape", in: "logo.png%zz", ok: false}, + {name: "empty path", in: "", ok: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := cleanPageImageRelativePath(tt.in) + if ok != tt.ok { + t.Fatalf("ok = %v, want %v", ok, tt.ok) + } + if got != tt.want { + t.Fatalf("path = %q, want %q", got, tt.want) + } + }) + } +} + +func TestResolvePageImagePath(t *testing.T) { + root := t.TempDir() + pagesDir := filepath.Join(root, "pages") + base := filepath.Join(pagesDir, "guide") + if err := os.MkdirAll(filepath.Join(base, "images"), 0755); err != nil { + t.Fatalf("create images dir: %v", err) + } + if err := os.WriteFile(filepath.Join(base, "logo.png"), []byte("fake"), 0644); err != nil { + t.Fatalf("create direct image: %v", err) + } + if err := os.WriteFile(filepath.Join(base, "images", "logo.png"), []byte("fake"), 0644); err != nil { + t.Fatalf("create image: %v", err) + } + + got, ok := resolvePageImagePath(pagesDir, base, "logo.png") + if !ok { + t.Fatal("expected direct image path to be accepted") + } + want := filepath.Join(base, "logo.png") + if got != want { + t.Fatalf("path = %q, want %q", got, want) + } + + got, ok = resolvePageImagePath(pagesDir, base, "images/logo.png") + if !ok { + t.Fatal("expected nested image path to be accepted") + } + want = filepath.Join(base, "images", "logo.png") + if got != want { + t.Fatalf("path = %q, want %q", got, want) + } + + if got, ok := resolvePageImagePath(pagesDir, base, "../guide.md"); ok { + t.Fatalf("expected traversal to be rejected, got %q", got) + } +} + +func TestResolvePageImagePathRejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + pagesDir := filepath.Join(root, "pages") + base := filepath.Join(pagesDir, "guide") + outside := filepath.Join(root, "outside") + + if err := os.MkdirAll(base, 0755); err != nil { + t.Fatalf("create page dir: %v", err) + } + if err := os.MkdirAll(outside, 0755); err != nil { + t.Fatalf("create outside dir: %v", err) + } + if err := os.WriteFile(filepath.Join(outside, "secret.png"), []byte("secret"), 0644); err != nil { + t.Fatalf("create outside file: %v", err) + } + if err := os.Symlink(outside, filepath.Join(base, "images")); err != nil { + t.Skipf("symlink not supported: %v", err) + } + + if got, ok := resolvePageImagePath(pagesDir, base, "images/secret.png"); ok { + t.Fatalf("expected symlink escape to be rejected, got %q", got) + } +} diff --git a/backend/internal/service/content_moderation_test.go b/backend/internal/service/content_moderation_test.go index 0c1a39c5..cc888f28 100644 --- a/backend/internal/service/content_moderation_test.go +++ b/backend/internal/service/content_moderation_test.go @@ -187,6 +187,14 @@ func (r *contentModerationTestUserRepo) UpdateConcurrency(ctx context.Context, i panic("unexpected UpdateConcurrency call") } +func (r *contentModerationTestUserRepo) BatchSetConcurrency(ctx context.Context, userIDs []int64, value int) (int, error) { + panic("unexpected BatchSetConcurrency call") +} + +func (r *contentModerationTestUserRepo) BatchAddConcurrency(ctx context.Context, userIDs []int64, delta int) (int, error) { + panic("unexpected BatchAddConcurrency call") +} + func (r *contentModerationTestUserRepo) ExistsByEmail(ctx context.Context, email string) (bool, error) { panic("unexpected ExistsByEmail call") } diff --git a/frontend/src/views/user/CustomPageView.vue b/frontend/src/views/user/CustomPageView.vue index 0752d5e3..59764c63 100644 --- a/frontend/src/views/user/CustomPageView.vue +++ b/frontend/src/views/user/CustomPageView.vue @@ -197,6 +197,29 @@ function generateHeadingId(text: string, index: number): string { return base ? `${base}-${index}` : `heading-${index}` } +function isRelativeMarkdownAsset(src: string): boolean { + const trimmed = src.trim() + if (!trimmed || /^[a-z][a-z0-9+.-]*:/i.test(trimmed) || trimmed.startsWith('//') || trimmed.startsWith('/')) { + return false + } + const [pathPart] = trimmed.split(/([?#].*)/, 2) + return pathPart + .split('/') + .filter((part) => part && part !== '.') + .every((part) => part !== '..' && !part.includes('\\')) +} + +function buildPageImageUrl(slug: string, src: string): string { + const trimmed = src.trim() + const [pathPart, suffix = ''] = trimmed.split(/([?#].*)/, 2) + const encodedPath = pathPart + .split('/') + .filter((part) => part && part !== '.') + .map((part) => encodeURIComponent(part)) + .join('/') + return `/api/v1/pages/${encodeURIComponent(slug)}/images/${encodedPath}${suffix}` +} + async function fetchAndRenderMarkdown(slug: string) { loading.value = true tocItems.value = [] @@ -212,8 +235,8 @@ async function fetchAndRenderMarkdown(slug: string) { let raw = await resp.text() raw = raw.replace( - /!\[([^\]]*)\]\((?!https?:\/\/)([^)]+)\)/g, - (_, alt, src) => `![${alt}](/api/v1/pages/${slug}/images/${src})` + /!\[([^\]]*)\]\(([^)]+)\)/g, + (match, alt, src) => isRelativeMarkdownAsset(src) ? `![${alt}](${buildPageImageUrl(slug, src)})` : match ) const html = marked.parse(raw) as string