From fe2aaa81cae9e91b67d3eaa99012732381478455 Mon Sep 17 00:00:00 2001 From: oyi77 Date: Wed, 1 Apr 2026 05:09:09 +0700 Subject: [PATCH] fix: address code review issues - Add file type validation for logo/favicon uploads - Add error state handling instead of alert() - Style file inputs with proper button appearance - Add SSRF protection to favicon API (URL validation) - Add fetch timeout (5 seconds) - Add content-type validation - Reduce cache duration to 5 minutes - Validate image data size before serving --- package-lock.json | 1 + .../settings/components/AppearanceTab.tsx | 123 ++++++++++++------ src/app/api/settings/favicon/route.ts | 114 ++++++++++++++-- 3 files changed, 186 insertions(+), 52 deletions(-) diff --git a/package-lock.json b/package-lock.json index c884d26fcb..d5cc101566 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10707,6 +10707,7 @@ "version": "2.3.2", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, "hasInstallScript": true, "license": "MIT", "optional": true, diff --git a/src/app/(dashboard)/dashboard/settings/components/AppearanceTab.tsx b/src/app/(dashboard)/dashboard/settings/components/AppearanceTab.tsx index 687b3f511c..89ef8242fd 100644 --- a/src/app/(dashboard)/dashboard/settings/components/AppearanceTab.tsx +++ b/src/app/(dashboard)/dashboard/settings/components/AppearanceTab.tsx @@ -21,6 +21,7 @@ export default function AppearanceTab() { const tSidebar = useTranslations("sidebar"); const [settings, setSettings] = useState>({}); const [loading, setLoading] = useState(true); + const [uploadError, setUploadError] = useState(null); const [customThemeColor, setCustomThemeColor] = useState(customColor || "#3b82f6"); const isValidHex = /^#([0-9a-fA-F]{6})$/.test( customThemeColor.startsWith("#") ? customThemeColor : `#${customThemeColor}` @@ -342,26 +343,47 @@ export default function AppearanceTab() {

{t("uploadLogo")}

- { - const file = e.target.files?.[0]; - if (file) { - if (file.size > 500 * 1024) { - alert("Logo file must be less than 500KB"); - return; +
+ {uploadError &&

{uploadError}

} {(settings.customLogoBase64 || settings.customLogoUrl) && (

{t("logoPreview")}

@@ -414,26 +437,47 @@ export default function AppearanceTab() {

{t("uploadFavicon")}

- { - const file = e.target.files?.[0]; - if (file) { - if (file.size > 50 * 1024) { - alert("Favicon file must be less than 50KB"); - return; +
+ {uploadError && !uploadError.includes("Logo") && ( +

{uploadError}

+ )} {(settings.customFaviconBase64 || settings.customFaviconUrl) && (

{t("faviconPreview")}

diff --git a/src/app/api/settings/favicon/route.ts b/src/app/api/settings/favicon/route.ts index de9390cba0..164f1410dc 100644 --- a/src/app/api/settings/favicon/route.ts +++ b/src/app/api/settings/favicon/route.ts @@ -3,29 +3,115 @@ import { getSettings } from "@/lib/db/settings"; export const dynamic = "force-dynamic"; +const ALLOWED_IMAGE_TYPES = [ + "image/png", + "image/x-icon", + "image/svg+xml", + "image/gif", + "image/webp", + "image/jpeg", +]; +const MAX_FAVICON_SIZE = 50 * 1024; // 50KB +const FETCH_TIMEOUT = 5000; // 5 seconds +const CACHE_DURATION = 300; // 5 minutes + +function isAllowedUrl(url: string): boolean { + try { + const parsedUrl = new URL(url); + // Only allow https (or http for local development) + if (parsedUrl.protocol !== "https:" && parsedUrl.protocol !== "http:") { + return false; + } + // Block private/internal IPs + const hostname = parsedUrl.hostname; + if ( + hostname === "localhost" || + hostname === "127.0.0.1" || + hostname === "0.0.0.0" || + hostname.startsWith("192.168.") || + hostname.startsWith("10.") || + hostname.startsWith("172.") || + hostname.endsWith(".local") || + hostname === "localhost" + ) { + return false; + } + return true; + } catch { + return false; + } +} + +function validateImageData(base64Data: string, contentType: string): boolean { + if (!ALLOWED_IMAGE_TYPES.includes(contentType)) { + console.error("Invalid content type:", contentType); + return false; + } + // Check for obvious image magic bytes + const matches = base64Data.match(/^data:[^;]+;base64,(.+)$/); + if (!matches) return false; + + const binaryData = Buffer.from(matches[1], "base64"); + if (binaryData.length > MAX_FAVICON_SIZE) { + console.error("Favicon too large:", binaryData.length); + return false; + } + + return true; +} + export async function GET() { try { const settings = await getSettings(); - const customFaviconBase64 = settings?.customFaviconBase64; - const customFaviconUrl = settings?.customFaviconUrl; + const customFaviconBase64 = settings?.customFaviconBase64 as string | undefined; + const customFaviconUrl = settings?.customFaviconUrl as string | undefined; let faviconData: string | null = null; if (customFaviconBase64) { - faviconData = customFaviconBase64; + // Validate stored Base64 data + const match = customFaviconBase64.match(/^data:([^;]+);base64,(.+)$/); + if (match && validateImageData(customFaviconBase64, match[1])) { + faviconData = customFaviconBase64; + } } else if (customFaviconUrl) { - try { - const response = await fetch(customFaviconUrl); - if (response.ok) { - const arrayBuffer = await response.arrayBuffer(); - const uint8Array = new Uint8Array(arrayBuffer); - const base64 = Buffer.from(uint8Array).toString("base64"); - const contentType = response.headers.get("content-type") || "image/png"; - faviconData = `data:${contentType};base64,${base64}`; + // Validate URL before fetching (SSRF protection) + if (!isAllowedUrl(customFaviconUrl)) { + console.error("Blocked invalid favicon URL:", customFaviconUrl); + } else { + try { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT); + + const response = await fetch(customFaviconUrl, { + signal: controller.signal, + headers: { + "User-Agent": "OmniRoute/1.0", + }, + }); + clearTimeout(timeoutId); + + if (response.ok) { + const contentType = response.headers.get("content-type") || ""; + const arrayBuffer = await response.arrayBuffer(); + const uint8Array = new Uint8Array(arrayBuffer); + + // Validate size before processing + if (uint8Array.length > MAX_FAVICON_SIZE) { + console.error("Favicon exceeds max size:", uint8Array.length); + } else { + const base64 = Buffer.from(uint8Array).toString("base64"); + const fullData = `data:${contentType};base64,${base64}`; + + if (validateImageData(fullData, contentType)) { + faviconData = fullData; + } + } + } + } catch (error) { + console.error("Failed to fetch custom favicon:", error); } - } catch (error) { - console.error("Failed to fetch custom favicon:", error); } } @@ -45,7 +131,7 @@ export async function GET() { return new NextResponse(buffer, { headers: { "Content-Type": contentType, - "Cache-Control": "public, max-age=3600", + "Cache-Control": `public, max-age=${CACHE_DURATION}`, }, }); } catch (error) {