security: fix CodeQL vulnerabilities (SSRF, XSS, rate limiting)

- Add URL validation to prevent SSRF attacks on notification endpoints - Block private IPs (10.x, 172.16-31.x, 192.168.x, 169.254.x) - Block localhost and internal hostnames - Only allow HTTP/HTTPS protocols - Add HTML escaping for medication names in email templates (XSS) - Add stricter rate limiting for auth routes (5 req/15min for login/register) - Add SSRF protection tests (405 tests total)
2025-12-30 11:52:00 +01:00
parent b5e12c7a95
commit cb1810586d
6 changed files with 138 additions and 9 deletions
@@ -13,7 +13,7 @@
        "@fastify/helmet": "^13.0.2",
        "@fastify/jwt": "^10.0.0",
        "@fastify/multipart": "^9.3.0",
-        "@fastify/rate-limit": "^10.1.0",
+        "@fastify/rate-limit": "^10.3.0",
        "@fastify/sensible": "^6.0.4",
        "@fastify/static": "^8.3.0",
        "@libsql/client": "^0.10.0",
@@ -18,7 +18,7 @@
    "@fastify/helmet": "^13.0.2",
    "@fastify/jwt": "^10.0.0",
    "@fastify/multipart": "^9.3.0",
-    "@fastify/rate-limit": "^10.1.0",
+    "@fastify/rate-limit": "^10.3.0",
    "@fastify/sensible": "^6.0.4",
    "@fastify/static": "^8.3.0",
    "@libsql/client": "^0.10.0",
@@ -20,6 +20,28 @@ const ARGON2_OPTIONS: argon2.Options = {
  hashLength: 32,              // 256-bit hash
 };

+// =============================================================================
+// Rate Limiting Configuration for Auth Routes
+// =============================================================================
+// Stricter rate limits for authentication endpoints to prevent brute-force attacks
+const authRateLimitConfig = {
+  max: 10,                     // 10 requests
+  timeWindow: "1 minute",      // per minute
+  errorResponseBuilder: () => ({
+    error: "Too many requests. Please try again later.",
+    code: "RATE_LIMIT_EXCEEDED",
+  }),
+};
+
+const sensitiveRateLimitConfig = {
+  max: 5,                      // 5 requests  
+  timeWindow: "15 minutes",    // per 15 minutes (for login/register)
+  errorResponseBuilder: () => ({
+    error: "Too many attempts. Please try again later.",
+    code: "RATE_LIMIT_EXCEEDED",
+  }),
+};
+
 // =============================================================================
 // Validation Schemas
 // =============================================================================
@@ -65,7 +87,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/register - User registration
  // ---------------------------------------------------------------------------
-  app.post<{ Body: z.infer<typeof registerSchema> }>("/auth/register", async (request, reply) => {
+  app.post<{ Body: z.infer<typeof registerSchema> }>("/auth/register", {
+    config: { rateLimit: sensitiveRateLimitConfig },
+  }, async (request, reply) => {
    // Check auth state
    const state = await getAuthState();
    
@@ -123,7 +147,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/login - User login
  // ---------------------------------------------------------------------------
-  app.post<{ Body: z.infer<typeof loginSchema> }>("/auth/login", async (request, reply) => {
+  app.post<{ Body: z.infer<typeof loginSchema> }>("/auth/login", {
+    config: { rateLimit: sensitiveRateLimitConfig },
+  }, async (request, reply) => {
    const state = await getAuthState();
    
    if (!state.authEnabled) {
@@ -223,7 +249,9 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // POST /auth/refresh - Refresh access token
  // ---------------------------------------------------------------------------
-  app.post("/auth/refresh", async (request, reply) => {
+  app.post("/auth/refresh", {
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const refreshTokenCookie = request.cookies.refresh_token;
    if (!refreshTokenCookie) {
      return reply.status(401).send({ error: "No refresh token", code: "NO_REFRESH_TOKEN" });
@@ -340,7 +368,10 @@ export async function authRoutes(app: FastifyInstance) {
  // ---------------------------------------------------------------------------
  // PUT /auth/me - Update current user profile
  // ---------------------------------------------------------------------------
-  app.put<{ Body: z.infer<typeof updateProfileSchema> }>("/auth/me", { preHandler: requireAuth }, async (request, reply) => {
+  app.put<{ Body: z.infer<typeof updateProfileSchema> }>("/auth/me", { 
+    preHandler: requireAuth,
+    config: { rateLimit: authRateLimitConfig },
+  }, async (request, reply) => {
    const authUser = request.user as unknown as AuthUser | null;
    if (!authUser) {
      return reply.status(401).send({ error: "Not authenticated" });
@@ -7,6 +7,18 @@ import type { AuthUser } from "../types/fastify.js";
 import { requireAuth, getAnonymousUserId } from "../plugins/auth.js";
 import { env } from "../plugins/env.js";

+// Escape HTML to prevent XSS in email templates
+function escapeHtml(text: string): string {
+  const htmlEscapes: Record<string, string> = {
+    '&': '&amp;',
+    '<': '&lt;',
+    '>': '&gt;',
+    '"': '&quot;',
+    "'": '&#39;',
+  };
+  return text.replace(/[&<>"']/g, char => htmlEscapes[char] || char);
+}
+
 type PlannerRow = {
  medicationId: number;
  medicationName: string;
@@ -100,7 +112,7 @@ export async function plannerRoutes(app: FastifyInstance) {
      .map(
        (row) => `
        <tr>
-          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${row.medicationName}</td>
+          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${escapeHtml(row.medicationName)}</td>
          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;"><strong>${row.totalPills}</strong></td>
          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;"><strong>${row.plannerUsage}</strong></td>
          <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;">${row.blistersNeeded} × ${row.blisterSize}</td>
@@ -281,7 +293,7 @@ Sent from MedAssist-ng Medication Planner`;
          const rowBg = isEmpty ? "#fef2f2" : "white";
          return `
          <tr style="background: ${rowBg};">
-            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${statusIcon} ${row.name}</td>
+            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; white-space: nowrap;">${statusIcon} ${escapeHtml(row.name)}</td>
            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap; ${isEmpty ? "color: #dc2626; font-weight: 600;" : ""}"><strong>${row.medsLeft}</strong></td>
            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;">${row.daysLeft ?? 0}</td>
            <td style="padding: 10px 12px; border-bottom: 1px solid #e5e7eb; text-align: center; white-space: nowrap;">${isEmpty ? "<strong>NOW</strong>" : (row.depletionDate ?? "-")}</td>
@@ -327,9 +327,61 @@ export async function settingsRoutes(app: FastifyInstance) {
  });
 }

+// Validate URL to prevent SSRF attacks
+function isAllowedNotificationUrl(urlStr: string): { allowed: boolean; error?: string } {
+  try {
+    // Convert ntfy:// to https:// for parsing
+    const normalizedUrl = urlStr.startsWith("ntfy://") 
+      ? urlStr.replace("ntfy://", "https://") 
+      : urlStr;
+    
+    const parsed = new URL(normalizedUrl);
+    
+    // Only allow http and https protocols
+    if (!['http:', 'https:'].includes(parsed.protocol)) {
+      return { allowed: false, error: "Only HTTP/HTTPS protocols are allowed" };
+    }
+    
+    // Block private/internal IP addresses
+    const hostname = parsed.hostname.toLowerCase();
+    
+    // Block localhost
+    if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') {
+      return { allowed: false, error: "Localhost URLs are not allowed" };
+    }
+    
+    // Block private IP ranges (basic check)
+    const ipMatch = hostname.match(/^(\d+)\.(\d+)\.(\d+)\.(\d+)$/);
+    if (ipMatch) {
+      const [, a, b] = ipMatch.map(Number);
+      // 10.x.x.x, 172.16-31.x.x, 192.168.x.x, 169.254.x.x (link-local)
+      if (a === 10 || a === 127 || (a === 172 && b >= 16 && b <= 31) || 
+          (a === 192 && b === 168) || (a === 169 && b === 254)) {
+        return { allowed: false, error: "Private IP addresses are not allowed" };
+      }
+    }
+    
+    // Block common internal hostnames
+    if (hostname.endsWith('.local') || hostname.endsWith('.internal') || 
+        hostname.endsWith('.lan') || hostname === 'metadata.google.internal') {
+      return { allowed: false, error: "Internal hostnames are not allowed" };
+    }
+    
+    return { allowed: true };
+  } catch {
+    return { allowed: false, error: "Invalid URL format" };
+  }
+}
+
 // Send notification via Shoutrrr-compatible URL (supports ntfy, Discord, Telegram, etc.)
 export async function sendShoutrrrNotification(urlStr: string, title: string, message: string): Promise<{ success: boolean; error?: string }> {
  try {
+    // Validate URL to prevent SSRF
+    const validation = isAllowedNotificationUrl(urlStr);
+    if (!validation.allowed) {
+      return { success: false, error: validation.error };
+    }
+    
    let targetUrl: string;
    let method = "POST";
    let headers: Record<string, string> = {};
@@ -1023,7 +1023,41 @@ describe("E2E Tests with Real Routes", () => {
      });

      expect(response.statusCode).toBe(500);
-      expect(response.json().error).toContain("Unsupported URL format");
+      // SSRF protection returns more specific error message
+      expect(response.json().error).toContain("HTTP/HTTPS protocols");
+    });
+
+    it("should reject test-shoutrrr with localhost URL (SSRF protection)", async () => {
+      const response = await app.inject({
+        method: "POST",
+        url: "/settings/test-shoutrrr",
+        payload: { url: "https://localhost/topic" },
+      });
+
+      expect(response.statusCode).toBe(500);
+      expect(response.json().error).toContain("Localhost URLs are not allowed");
+    });
+
+    it("should reject test-shoutrrr with private IP (SSRF protection)", async () => {
+      const response = await app.inject({
+        method: "POST",
+        url: "/settings/test-shoutrrr",
+        payload: { url: "https://192.168.1.1/topic" },
+      });
+
+      expect(response.statusCode).toBe(500);
+      expect(response.json().error).toContain("Private IP addresses are not allowed");
+    });
+
+    it("should reject test-shoutrrr with internal hostname (SSRF protection)", async () => {
+      const response = await app.inject({
+        method: "POST",
+        url: "/settings/test-shoutrrr",
+        payload: { url: "https://server.internal/topic" },
+      });
+
+      expect(response.statusCode).toBe(500);
+      expect(response.json().error).toContain("Internal hostnames are not allowed");
    });
  });