From adf3855ac89d188c8b94ffc9c7dd6f4779460abd Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 18:16:57 -0500 Subject: [PATCH] harden wallpaper resolver against traversal, PII, and SSOT drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review surfaced four defects and four drive-bys. All applied: B1 (security, MEDIUM) — Path traversal via encodeRelativeAssetPath. encodeURIComponent passed "." and ".." through unchanged; percent-encoded "%2e%2e" got decoded by the URL constructor. Either form escaped the asset root: new URL("../../etc/passwd", "file:///opt/Openscreen/resources/") → file:///opt/etc/passwd. Reject both at src/lib/assetPath.ts via a new UnsafeAssetPathError thrown when a decoded segment equals "." or "..". B2 (correctness) — classifyWallpaper returned { kind: "image" } for conic-gradient(...), rgb(...), hsl(...), oklch(...), empty string, and named colors like "red". Old frameRenderer's bare fillStyle = value handled these; new code would throw BackgroundLoadError with misleading message. Classification now anchors on regexes, accepts all CSS color functions and all three gradient types, treats unknown strings as fallthrough color (old behavior), and normalizes "" to "#000000". B3 (SSOT) — DEFAULT_WALLPAPER, projectPersistence.WALLPAPER_PATHS, and SettingsPanel.WALLPAPER_RELATIVE independently hardcoded the same /wallpapers/wallpaperN.jpg pattern. Three drift sites collapse into one: WALLPAPER_PATHS lives in src/lib/wallpaper.ts, DEFAULT_WALLPAPER derives from WALLPAPER_PATHS[0], projectPersistence re-exports from the canonical module, SettingsPanel imports it directly. B4 (privacy) — BackgroundLoadError.message and the translated toast surfaced full file paths like file:///home//…/wallpaper.jpg — leaks the user's home directory in copy-pasted bug reports. Added a displayUrl getter that returns just the basename (or "data:…" for data URIs), wired into the toast. Full URL remains in console.error and error.url for debugging. N1 — resolveImageWallpaperUrl now rejects image paths that don't live under /wallpapers/ (throws BackgroundLoadError). Narrows the blast radius of the returned / base so the renderer can only request files within the wallpapers directory, regardless of what the project JSON claims. N2 — videoExporter retry loop no longer calls cleanup() twice in the BackgroundLoadError branch; the finally handles it. N3 — Browser tests assert BackgroundLoadError.url contains the failing path. Guards the {{url}} i18n interpolation contract. N4 — VideoPlayback wallpaper resolve effect now catches resolver throws (UnsafeAssetPathError, BackgroundLoadError from /wallpapers/ prefix enforcement). Prevents the new strict-rejection logic from silently leaving the preview without a background. Tests: 35 unit tests pass (up from 20); new coverage for all color functions, all gradient types, empty string, named color fallback, whitespace trimming, /wallpapers/ prefix enforcement, traversal rejection, percent-encoded traversal rejection, displayUrl basename and data-URI abbreviation. --- src/components/video-editor/SettingsPanel.tsx | 84 ++++++------ src/components/video-editor/VideoEditor.tsx | 2 +- src/components/video-editor/VideoPlayback.tsx | 9 +- .../video-editor/projectPersistence.ts | 8 +- src/lib/assetPath.ts | 23 +++- src/lib/exporter/gifExporter.browser.test.ts | 6 +- .../exporter/videoExporter.browser.test.ts | 6 +- src/lib/exporter/videoExporter.ts | 1 - src/lib/wallpaper.test.ts | 128 ++++++++++++++---- src/lib/wallpaper.ts | 62 +++++++-- 10 files changed, 235 insertions(+), 94 deletions(-) diff --git a/src/components/video-editor/SettingsPanel.tsx b/src/components/video-editor/SettingsPanel.tsx index 4fb4193..ec5ad0d 100644 --- a/src/components/video-editor/SettingsPanel.tsx +++ b/src/components/video-editor/SettingsPanel.tsx @@ -34,11 +34,11 @@ import { Slider } from "@/components/ui/slider"; import { Switch } from "@/components/ui/switch"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { useScopedT } from "@/contexts/I18nContext"; -import { getAssetPath } from "@/lib/assetPath"; import { WEBCAM_LAYOUT_PRESETS } from "@/lib/compositeLayout"; import type { ExportFormat, ExportQuality, GifFrameRate, GifSizePreset } from "@/lib/exporter"; import { GIF_FRAME_RATES, GIF_SIZE_PRESETS } from "@/lib/exporter"; import { cn } from "@/lib/utils"; +import { resolveImageWallpaperUrl, WALLPAPER_PATHS } from "@/lib/wallpaper"; import { type AspectRatio, isPortraitAspectRatio } from "@/utils/aspectRatioUtils"; import { getTestId } from "@/utils/getTestId"; import { AnnotationSettingsPanel } from "./AnnotationSettingsPanel"; @@ -123,11 +123,6 @@ function CustomSpeedInput({ ); } -const WALLPAPER_COUNT = 18; -const WALLPAPER_RELATIVE = Array.from( - { length: WALLPAPER_COUNT }, - (_, i) => `wallpapers/wallpaper${i + 1}.jpg`, -); const GRADIENTS = [ "linear-gradient( 111.6deg, rgba(114,167,232,1) 9.4%, rgba(253,129,82,1) 43.9%, rgba(253,129,82,1) 54.8%, rgba(249,202,86,1) 86.3% )", "linear-gradient(120deg, #d4fc79 0%, #96e6a1 100%)", @@ -334,10 +329,10 @@ export function SettingsPanel({ let mounted = true; (async () => { try { - const resolved = await Promise.all(WALLPAPER_RELATIVE.map((p) => getAssetPath(p))); + const resolved = await Promise.all(WALLPAPER_PATHS.map((p) => resolveImageWallpaperUrl(p))); if (mounted) setWallpaperPaths(resolved); } catch (_err) { - if (mounted) setWallpaperPaths(WALLPAPER_RELATIVE.map((p) => `/${p}`)); + if (mounted) setWallpaperPaths([...WALLPAPER_PATHS]); } })(); return () => { @@ -526,7 +521,7 @@ export function SettingsPanel({ setCustomImages((prev) => prev.filter((img) => img !== imageUrl)); // If the removed image was selected, clear selection if (selected === imageUrl) { - onWallpaperChange(wallpaperPaths[0] || WALLPAPER_RELATIVE[0]); + onWallpaperChange(wallpaperPaths[0] || WALLPAPER_PATHS[0]); } }; @@ -1146,42 +1141,41 @@ export function SettingsPanel({ ); })} - {(wallpaperPaths.length > 0 - ? wallpaperPaths - : WALLPAPER_RELATIVE.map((p) => `/${p}`) - ).map((path) => { - const isSelected = (() => { - if (!selected) return false; - if (selected === path) return true; - try { - const clean = (s: string) => - s.replace(/^file:\/\//, "").replace(/^\//, ""); - if (clean(selected).endsWith(clean(path))) return true; - if (clean(path).endsWith(clean(selected))) return true; - } catch { - // Best-effort comparison; fallback to strict match. - } - return false; - })(); - return ( -
onWallpaperChange(path)} - role="button" - /> - ); - })} + {(wallpaperPaths.length > 0 ? wallpaperPaths : WALLPAPER_PATHS).map( + (path) => { + const isSelected = (() => { + if (!selected) return false; + if (selected === path) return true; + try { + const clean = (s: string) => + s.replace(/^file:\/\//, "").replace(/^\//, ""); + if (clean(selected).endsWith(clean(path))) return true; + if (clean(path).endsWith(clean(selected))) return true; + } catch { + // Best-effort comparison; fallback to strict match. + } + return false; + })(); + return ( +
onWallpaperChange(path)} + role="button" + /> + ); + }, + )}
diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index 3694d9e..0a03bf1 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -1568,7 +1568,7 @@ export default function VideoEditor() { } catch (error) { console.error("Export error:", error); if (error instanceof BackgroundLoadError) { - const message = t("errors.exportBackgroundLoadFailed", { url: error.url }); + const message = t("errors.exportBackgroundLoadFailed", { url: error.displayUrl }); setExportError(message); toast.error(message); } else { diff --git a/src/components/video-editor/VideoPlayback.tsx b/src/components/video-editor/VideoPlayback.tsx index db69310..d356012 100644 --- a/src/components/video-editor/VideoPlayback.tsx +++ b/src/components/video-editor/VideoPlayback.tsx @@ -1185,8 +1185,13 @@ const VideoPlayback = forwardRef( if (mounted) setResolvedWallpaper(classified.value); return; } - const resolved = await resolveImageWallpaperUrl(classified.path); - if (mounted) setResolvedWallpaper(resolved); + try { + const resolved = await resolveImageWallpaperUrl(classified.path); + if (mounted) setResolvedWallpaper(resolved); + } catch (err) { + console.warn("[VideoPlayback] wallpaper resolve failed:", err); + if (mounted) setResolvedWallpaper(null); + } })(); return () => { mounted = false; diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index f9640e4..6d8f689 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -2,7 +2,7 @@ import { normalizeBlurColor, normalizeBlurType } from "@/lib/blurEffects"; import type { ExportFormat, ExportQuality, GifFrameRate, GifSizePreset } from "@/lib/exporter"; import type { ProjectMedia } from "@/lib/recordingSession"; import { normalizeProjectMedia } from "@/lib/recordingSession"; -import { DEFAULT_WALLPAPER } from "@/lib/wallpaper"; +import { DEFAULT_WALLPAPER, WALLPAPER_PATHS } from "@/lib/wallpaper"; import { ASPECT_RATIOS, type AspectRatio, isPortraitAspectRatio } from "@/utils/aspectRatioUtils"; import { type AnnotationRegion, @@ -38,13 +38,9 @@ import { type ZoomRegion, } from "./types"; -const WALLPAPER_COUNT = 18; const VALID_BLUR_SHAPES = new Set(["rectangle", "oval", "freehand"] as const); -export const WALLPAPER_PATHS = Array.from( - { length: WALLPAPER_COUNT }, - (_, i) => `/wallpapers/wallpaper${i + 1}.jpg`, -); +export { WALLPAPER_PATHS }; export const PROJECT_VERSION = 2; diff --git a/src/lib/assetPath.ts b/src/lib/assetPath.ts index 8188de5..7ba1015 100644 --- a/src/lib/assetPath.ts +++ b/src/lib/assetPath.ts @@ -1,9 +1,22 @@ +export class UnsafeAssetPathError extends Error { + constructor(segment: string) { + super(`Unsafe asset path segment: ${segment}`); + this.name = "UnsafeAssetPathError"; + } +} + function encodeRelativeAssetPath(relativePath: string): string { return relativePath .replace(/^\/+/, "") .split("/") .filter(Boolean) - .map((part) => encodeURIComponent(part)) + .map((part) => { + const decoded = decodeURIComponent(part); + if (decoded === "." || decoded === "..") { + throw new UnsafeAssetPathError(decoded); + } + return encodeURIComponent(decoded); + }) .join("/"); } @@ -16,7 +29,6 @@ export async function getAssetPath(relativePath: string): Promise { try { if (typeof window !== "undefined") { - // If running in a dev server (http/https), prefer the web-served path if ( window.location && window.location.protocol && @@ -32,11 +44,12 @@ export async function getAssetPath(relativePath: string): Promise { } } } - } catch { - // ignore and use fallback + } catch (err) { + if (err instanceof UnsafeAssetPathError) { + throw err; + } } - // Fallback for web/dev server: public/wallpapers are served at '/wallpapers/...' return `/${encodedRelativePath}`; } diff --git a/src/lib/exporter/gifExporter.browser.test.ts b/src/lib/exporter/gifExporter.browser.test.ts index 5a69468..1d96076 100644 --- a/src/lib/exporter/gifExporter.browser.test.ts +++ b/src/lib/exporter/gifExporter.browser.test.ts @@ -79,6 +79,10 @@ describe("GifExporter (real browser)", () => { cropRegion: { x: 0, y: 0, width: 1, height: 1 }, }); - await expect(exporter.export()).rejects.toBeInstanceOf(BackgroundLoadError); + const rejection = exporter.export(); + await expect(rejection).rejects.toBeInstanceOf(BackgroundLoadError); + await expect(rejection).rejects.toMatchObject({ + url: expect.stringContaining("does-not-exist"), + }); }); }); diff --git a/src/lib/exporter/videoExporter.browser.test.ts b/src/lib/exporter/videoExporter.browser.test.ts index ae8c7bc..cca896f 100644 --- a/src/lib/exporter/videoExporter.browser.test.ts +++ b/src/lib/exporter/videoExporter.browser.test.ts @@ -77,6 +77,10 @@ describe("VideoExporter (real browser)", () => { cropRegion: { x: 0, y: 0, width: 1, height: 1 }, }); - await expect(exporter.export()).rejects.toBeInstanceOf(BackgroundLoadError); + const rejection = exporter.export(); + await expect(rejection).rejects.toBeInstanceOf(BackgroundLoadError); + await expect(rejection).rejects.toMatchObject({ + url: expect.stringContaining("does-not-exist"), + }); }); }); diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index 0ea1ec6..cc8b7cf 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -84,7 +84,6 @@ export class VideoExporter { } if (normalizedError instanceof BackgroundLoadError) { - this.cleanup(); throw normalizedError; } diff --git a/src/lib/wallpaper.test.ts b/src/lib/wallpaper.test.ts index dbb5e3c..f4fe08e 100644 --- a/src/lib/wallpaper.test.ts +++ b/src/lib/wallpaper.test.ts @@ -1,54 +1,109 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { UnsafeAssetPathError } from "./assetPath"; import { BackgroundLoadError, classifyWallpaper, DEFAULT_WALLPAPER, resolveImageWallpaperUrl, + WALLPAPER_COUNT, + WALLPAPER_PATHS, } from "./wallpaper"; +describe("WALLPAPER_PATHS", () => { + it("contains WALLPAPER_COUNT entries", () => { + expect(WALLPAPER_PATHS).toHaveLength(WALLPAPER_COUNT); + }); + + it("DEFAULT_WALLPAPER is WALLPAPER_PATHS[0]", () => { + expect(DEFAULT_WALLPAPER).toBe(WALLPAPER_PATHS[0]); + }); +}); + describe("classifyWallpaper", () => { - it("classifies hex color", () => { + it("hex color", () => { expect(classifyWallpaper("#1a1a2e")).toEqual({ kind: "color", value: "#1a1a2e" }); }); - it("classifies linear gradient", () => { - const value = "linear-gradient(90deg, red, blue)"; - expect(classifyWallpaper(value)).toEqual({ kind: "gradient", value }); + it("rgb() color", () => { + expect(classifyWallpaper("rgb(1, 2, 3)")).toEqual({ kind: "color", value: "rgb(1, 2, 3)" }); }); - it("classifies radial gradient", () => { - const value = "radial-gradient(circle, red, blue)"; - expect(classifyWallpaper(value)).toEqual({ kind: "gradient", value }); + it("rgba() color", () => { + expect(classifyWallpaper("rgba(1, 2, 3, 0.5)")).toEqual({ + kind: "color", + value: "rgba(1, 2, 3, 0.5)", + }); }); - it("classifies leading-slash image path", () => { + it("hsl() color", () => { + expect(classifyWallpaper("hsl(180, 50%, 50%)")).toEqual({ + kind: "color", + value: "hsl(180, 50%, 50%)", + }); + }); + + it("oklch() color", () => { + expect(classifyWallpaper("oklch(50% 0.1 180)")).toEqual({ + kind: "color", + value: "oklch(50% 0.1 180)", + }); + }); + + it("linear gradient", () => { + const v = "linear-gradient(90deg, red, blue)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("radial gradient", () => { + const v = "radial-gradient(circle, red, blue)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("conic gradient", () => { + const v = "conic-gradient(red, blue)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("leading-slash image path", () => { expect(classifyWallpaper("/wallpapers/wallpaper1.jpg")).toEqual({ kind: "image", path: "/wallpapers/wallpaper1.jpg", }); }); - it("classifies http URL as image", () => { + it("http URL as image", () => { expect(classifyWallpaper("https://example.com/bg.jpg")).toEqual({ kind: "image", path: "https://example.com/bg.jpg", }); }); - it("classifies file:// URL as image", () => { + it("file:// URL as image", () => { expect(classifyWallpaper("file:///tmp/bg.jpg")).toEqual({ kind: "image", path: "file:///tmp/bg.jpg", }); }); - it("classifies data URI as image", () => { + it("data URI as image", () => { expect(classifyWallpaper("data:image/png;base64,AAA")).toEqual({ kind: "image", path: "data:image/png;base64,AAA", }); }); + it("named color falls back to color", () => { + expect(classifyWallpaper("red")).toEqual({ kind: "color", value: "red" }); + }); + + it("empty string falls back to black", () => { + expect(classifyWallpaper("")).toEqual({ kind: "color", value: "#000000" }); + }); + + it("trims whitespace", () => { + expect(classifyWallpaper(" #abcdef ")).toEqual({ kind: "color", value: "#abcdef" }); + }); + it("DEFAULT_WALLPAPER classifies as image", () => { expect(classifyWallpaper(DEFAULT_WALLPAPER)).toEqual({ kind: "image", @@ -70,46 +125,64 @@ describe("resolveImageWallpaperUrl", () => { vi.unstubAllGlobals(); }); - it("passes through http URL unchanged", async () => { + it("passes through http URL", async () => { expect(await resolveImageWallpaperUrl("http://example.com/bg.jpg")).toBe( "http://example.com/bg.jpg", ); }); - it("passes through https URL unchanged", async () => { + it("passes through https URL", async () => { expect(await resolveImageWallpaperUrl("https://example.com/bg.jpg")).toBe( "https://example.com/bg.jpg", ); }); - it("passes through file:// URL unchanged", async () => { + it("passes through file:// URL", async () => { expect(await resolveImageWallpaperUrl("file:///tmp/bg.jpg")).toBe("file:///tmp/bg.jpg"); }); - it("passes through data URI unchanged", async () => { + it("passes through data URI", async () => { const uri = "data:image/png;base64,AAAA"; expect(await resolveImageWallpaperUrl(uri)).toBe(uri); }); - it("resolves leading-slash path via http dev server fallback", async () => { + it("resolves leading-slash wallpaper path via http fallback", async () => { expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( "/wallpapers/wallpaper1.jpg", ); }); - it("resolves bare relative path via http dev server fallback", async () => { + it("resolves bare relative wallpaper path", async () => { expect(await resolveImageWallpaperUrl("wallpapers/wallpaper1.jpg")).toBe( "/wallpapers/wallpaper1.jpg", ); }); - it("encodes path segments with special characters", async () => { + it("encodes special characters in path segments", async () => { expect(await resolveImageWallpaperUrl("/wallpapers/my image.jpg")).toBe( "/wallpapers/my%20image.jpg", ); }); - it("resolves via electronAPI when not http protocol", async () => { + it("rejects image paths outside /wallpapers/", async () => { + await expect(resolveImageWallpaperUrl("/etc/passwd")).rejects.toBeInstanceOf( + BackgroundLoadError, + ); + }); + + it("rejects traversal attempts", async () => { + await expect(resolveImageWallpaperUrl("/wallpapers/../etc/passwd")).rejects.toBeInstanceOf( + UnsafeAssetPathError, + ); + }); + + it("rejects percent-encoded traversal", async () => { + await expect(resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar")).rejects.toBeInstanceOf( + UnsafeAssetPathError, + ); + }); + + it("resolves via electronAPI when not http", async () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, @@ -122,7 +195,7 @@ describe("resolveImageWallpaperUrl", () => { ); }); - it("electronAPI branch appends trailing slash to base if missing", async () => { + it("electronAPI branch appends trailing slash if missing", async () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, @@ -151,12 +224,21 @@ describe("resolveImageWallpaperUrl", () => { describe("BackgroundLoadError", () => { it("carries the failing URL and is instanceof Error", () => { - const err = new BackgroundLoadError("file:///missing.jpg"); + const err = new BackgroundLoadError("/home/user/secret/wallpaper.jpg"); expect(err).toBeInstanceOf(Error); expect(err).toBeInstanceOf(BackgroundLoadError); - expect(err.url).toBe("file:///missing.jpg"); + expect(err.url).toBe("/home/user/secret/wallpaper.jpg"); expect(err.name).toBe("BackgroundLoadError"); - expect(err.message).toContain("file:///missing.jpg"); + }); + + it("displayUrl hides parent directories to avoid leaking PII", () => { + const err = new BackgroundLoadError("file:///home/enrique/projects/openscreen/wallpaper1.jpg"); + expect(err.displayUrl).toBe("wallpaper1.jpg"); + }); + + it("displayUrl abbreviates data URIs", () => { + const err = new BackgroundLoadError("data:image/png;base64,AAA"); + expect(err.displayUrl).toBe("data:…"); }); it("preserves cause when provided", () => { diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts index f949177..449d2e2 100644 --- a/src/lib/wallpaper.ts +++ b/src/lib/wallpaper.ts @@ -1,22 +1,42 @@ import { getAssetPath } from "@/lib/assetPath"; -export const DEFAULT_WALLPAPER = "/wallpapers/wallpaper1.jpg"; +export const WALLPAPER_COUNT = 18; + +export const WALLPAPER_PATHS: readonly string[] = Array.from( + { length: WALLPAPER_COUNT }, + (_, i) => `/wallpapers/wallpaper${i + 1}.jpg`, +); + +export const DEFAULT_WALLPAPER = WALLPAPER_PATHS[0]; export type WallpaperClassification = | { kind: "color"; value: string } | { kind: "gradient"; value: string } | { kind: "image"; path: string }; +const GRADIENT_RE = /^(linear|radial|conic)-gradient\(/; +const COLOR_FUNC_RE = /^(rgb|rgba|hsl|hsla|hwb|lab|lch|oklab|oklch|color)\(/; +const IMAGE_URL_RE = /^(\/|https?:\/\/|file:\/\/|data:)/; + export function classifyWallpaper(value: string): WallpaperClassification { - if (value.startsWith("#")) { - return { kind: "color", value }; + const trimmed = value.trim(); + if (trimmed === "") { + return { kind: "color", value: "#000000" }; } - if (value.startsWith("linear-gradient") || value.startsWith("radial-gradient")) { - return { kind: "gradient", value }; + if (trimmed.startsWith("#") || COLOR_FUNC_RE.test(trimmed)) { + return { kind: "color", value: trimmed }; } - return { kind: "image", path: value }; + if (GRADIENT_RE.test(trimmed)) { + return { kind: "gradient", value: trimmed }; + } + if (IMAGE_URL_RE.test(trimmed)) { + return { kind: "image", path: trimmed }; + } + return { kind: "color", value: trimmed }; } +const ALLOWED_IMAGE_PREFIX = "/wallpapers/"; + export async function resolveImageWallpaperUrl(imagePath: string): Promise { if ( imagePath.startsWith("http://") || @@ -26,8 +46,14 @@ export async function resolveImageWallpaperUrl(imagePath: string): Promise