fix: resolve IDOR vulnerability exposing private user data (#1853)

backend/config.js

@@ -7,13 +7,14 @@ const envSchema = z.object({
   SUPABASE_URL: z.string().url().optional(),
   SUPABASE_SERVICE_ROLE_KEY: z.string().optional(),
   SUPABASE_ANON_KEY: z.string().optional(),
+  SUPABASE_JWT_SECRET: z.string().min(1, "SUPABASE_JWT_SECRET is required for JWT verification").optional(),
   OPENROUTER_API_KEY: z.string().min(1),
   PASSWORD_RESET_BASE_URL: z.string().url().optional(),
   FRONTEND_URL: z.string().url().optional(),
   CLIENT_URL: z.string().url().optional(),
   EMAIL_USER: z.string().optional(),
   EMAIL_PASS: z.string().optional(),
-  SITE_URL: z.string().url().optional(),
+  SITE_URL: z.string().optional(),
 });
 
 const _env = envSchema.safeParse(process.env);

backend/controllers/notificationController.js

@@ -3,6 +3,10 @@ import webpush from "web-push";
 import { sanitizeNotificationActionUrl } from "../utils/notificationActionUrl.js";
 import { collectExpiredSubscriptionIds } from "../utils/pushDeliveryCleanup.js";
 
+// Strict UUID v4 validation to prevent injection attacks
+const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+const isValidUUID = (str) => typeof str === "string" && UUID_REGEX.test(str);
+
 const getSupabaseClient = () => {
   const supabaseUrl = process.env.SUPABASE_URL || process.env.VITE_SUPABASE_URL;
   const serviceRoleKey = process.env.SUPABASE_SERVICE_ROLE_KEY;
@@ -43,17 +47,31 @@ export const sendPushNotification = async (req, res, next) => {
         error: "Invalid request payload",
       });
     }
+
+    // Strict UUID validation on user_id to prevent injection and malformed input
+    if (!isValidUUID(user_id)) {
+      return res.status(400).json({
+        error: "Invalid user_id format",
+      });
+    }
+
     if (title.length > 100 || body.length > 500) {
       return res.status(400).json({
         error: "Notification content too long",
       });
     }
 
-    // Security Fix: Prevent IDOR. Enforce that standard users can only send push notifications to themselves.
-    // If a webhook secret is used, req.user will be undefined (which bypasses this check if we allow webhooks to send to anyone).
-    // If user auth is used, req.user is set.
-    const isAdmin = req.user?.role === "admin" || req.user?.app_metadata?.role === "admin" || req.roles?.includes("admin");
-    if (req.user?.id && req.user.id !== user_id && !isAdmin) {
+    // Security Fix (IDOR #1853): Enforce that standard users can ONLY send
+    // push notifications to themselves. Admins and webhook-authenticated
+    // callers (req.user is undefined) may target any user_id.
+    const isAdmin = req.user?.role === "admin"
+      || req.user?.app_metadata?.role === "admin"
+      || req.roles?.includes("admin");
+
+    if (req.user?.id && !isAdmin && req.user.id !== user_id) {
+      console.warn(
+        `[security] IDOR blocked: user ${req.user.id} attempted to send push notification to ${user_id}`
+      );
       return res.status(403).json({ error: "Not authorized to send push notifications to this user" });
     }
 

backend/middlewares/requireAuth.js

@@ -2,6 +2,8 @@ import crypto from "crypto";
 import { HttpError } from "../utils/httpError.js";
 import { getSupabaseAdmin } from "../utils/supabase.js";
 
+const ALLOWED_ALGORITHMS = ["HS256"];
+
 const base64UrlDecode = (str) => {
   str = str.replace(/-/g, "+").replace(/_/g, "/");
   while (str.length % 4) {
@@ -10,6 +12,14 @@ const base64UrlDecode = (str) => {
   return Buffer.from(str, "base64").toString("utf-8");
 };
 
+const constantTimeCompare = (a, b) => {
+  if (typeof a !== "string" || typeof b !== "string") return false;
+  if (a.length !== b.length) return false;
+  const bufA = Buffer.from(a);
+  const bufB = Buffer.from(b);
+  return crypto.timingSafeEqual(bufA, bufB);
+};
+
 const verifyLocalJwt = (token, secret) => {
   try {
     const parts = token.split(".");
@@ -18,13 +28,15 @@ const verifyLocalJwt = (token, secret) => {
     const [headerB64, payloadB64, signatureB64] = parts;
 
     const header = JSON.parse(base64UrlDecode(headerB64));
-
-    // Prevent algorithm confusion: Only process HS256 tokens using HMAC.
-    if (header.alg !== "HS256") {
+
+    if (!header || typeof header.alg !== "string") {
       return null;
     }
-
-    // Additional check: if the secret appears to be a PEM-encoded public key, reject HMAC
+
+    if (!ALLOWED_ALGORITHMS.includes(header.alg)) {
+      return null;
+    }
+
     if (secret.startsWith("-----BEGIN")) {
       return null;
     }
@@ -37,19 +49,27 @@ const verifyLocalJwt = (token, secret) => {
       .replace(/\//g, "_")
       .replace(/=/g, "");
 
-    const expectedSignatureBuffer = Buffer.from(expectedSignature);
-    const signatureBuffer = Buffer.from(signatureB64);
-
-    if (
-      expectedSignatureBuffer.length !== signatureBuffer.length ||
-      !crypto.timingSafeEqual(expectedSignatureBuffer, signatureBuffer)
-    ) {
+    if (!constantTimeCompare(expectedSignature, signatureB64)) {
       return null;
     }
 
     const payload = JSON.parse(base64UrlDecode(payloadB64));
 
-    if (payload.exp && Date.now() >= payload.exp * 1000) {
+    const now = Math.floor(Date.now() / 1000);
+
+    if (typeof payload.exp === "number" && now >= payload.exp) {
+      return null;
+    }
+
+    if (typeof payload.iat === "number" && now < payload.iat) {
+      return null;
+    }
+
+    if (payload.iss && payload.iss !== "supabase") {
+      return null;
+    }
+
+    if (payload.aud && !Array.isArray(payload.aud) && typeof payload.aud !== "string") {
       return null;
     }
 
@@ -71,7 +91,6 @@ if (!jwtSecret && isProduction) {
   process.exit(1);
 }
 
-// Rate limiter specifically for the slow fallback path
 const FALLBACK_WINDOW_MS = 60_000;
 const FALLBACK_MAX_REQUESTS = 10;
 const fallbackRateCounts = new Map();
@@ -120,7 +139,6 @@ export const requireAuth = async (req, res, next) => {
   }
 
   if (jwtSecret) {
-    // LOCAL HMAC verification - no network call
     const payload = verifyLocalJwt(token, jwtSecret);
     if (!payload) {
       next(new HttpError(401, "Invalid or expired session"));
@@ -132,14 +150,13 @@ export const requireAuth = async (req, res, next) => {
       email: payload.email,
       user_metadata: payload.user_metadata,
       app_metadata: payload.app_metadata,
-      role: payload.role
+      role: payload.role,
     };
     return next();
   }
 
-  // DEVELOPMENT ONLY FALLBACK
   console.warn("[security] Using slow network fallback for JWT verification. Do not use in production.");
-  
+
   const clientIp = req.socket?.remoteAddress || req.ip || "unknown";
   if (isFallbackRateLimited(clientIp)) {
     next(new HttpError(429, "Too many verification requests. Please try again later."));
@@ -154,7 +171,7 @@ export const requireAuth = async (req, res, next) => {
     }
 
     const { data: { user }, error } = await supabaseAdmin.auth.getUser(token);
-    
+
     if (error || !user) {
       next(new HttpError(401, "Invalid or expired session"));
       return;
@@ -237,3 +254,62 @@ export const requireProfileRole = (...allowedRoles) => async (req, res, next) =>
  * Any request missing the is_admin=true flag in the database will be rejected with 403.
  */
 export const requireAdminRole = requireProfileRole("admin");
+
+/**
+ * Middleware that enforces resource ownership.
+ *
+ * Extracts the resource owner's ID from one of three sources (in order):
+ *   1. req.params.<paramName>  – URL parameter (e.g. /api/users/:userId)
+ *   2. req.body.<bodyField>   – request body field
+ *   3. req.query.<queryField>  – query string parameter
+ *
+ * Then compares it against the authenticated user's ID (req.user.id).
+ * Admins and the resource owner are allowed through.
+ *
+ * @param {Object} options
+ * @param {string}  options.paramName   - Name of the URL param containing the owner ID
+ * @param {string}  [options.bodyField] - Name of the body field (fallback)
+ * @param {string}  [options.queryField] - Name of the query field (fallback)
+ * @returns {Function} Express middleware
+ *
+ * @example
+ *   router.get('/:userId/profile', requireAuth, requireOwnershipOrAdmin({ paramName: 'userId' }), handler);
+ */
+export const requireOwnershipOrAdmin = ({ paramName, bodyField, queryField } = {}) => async (req, res, next) => {
+  if (!req.user?.id) {
+    return next(new HttpError(401, "Authentication required"));
+  }
+
+  // Check if user has admin role (admins bypass ownership check)
+  const isAdmin = req.user?.role === "admin"
+    || req.user?.app_metadata?.role === "admin"
+    || req.roles?.includes("admin");
+
+  if (isAdmin) {
+    return next();
+  }
+
+  // Resolve the target user ID from params > body > query
+  const targetUserId = (paramName && req.params?.[paramName])
+    || (bodyField && req.body?.[bodyField])
+    || (queryField && req.query?.[queryField]);
+
+  if (!targetUserId) {
+    return next(new HttpError(400, "Missing resource identifier"));
+  }
+
+  // Strict UUID format check to prevent injection
+  const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+  if (!uuidRegex.test(targetUserId)) {
+    return next(new HttpError(400, "Invalid resource identifier format"));
+  }
+
+  if (req.user.id !== targetUserId) {
+    console.warn(
+      `[security] IDOR blocked: user ${req.user.id} attempted to access resource owned by ${targetUserId}`
+    );
+    return next(new HttpError(403, "Not authorized to access this resource"));
+  }
+
+  next();
+};

backend/routes/users.js

@@ -4,7 +4,8 @@ import path from "path";
 import fs from "fs";
 import { fileURLToPath } from "url";
 import { fileTypeFromFile } from "file-type";
-import { requireAuth } from "../middlewares/requireAuth.js";
+import { requireAuth, requireOwnershipOrAdmin } from "../middlewares/requireAuth.js";
+import { getSupabaseAdmin } from "../utils/supabase.js";
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
@@ -72,6 +73,121 @@ const safeUnlink = (filePath) => {
   }
 };
 
+// Strict UUID validation for URL parameters
+const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
+
+// Fields that are safe to expose publicly (non-sensitive profile data)
+const PUBLIC_PROFILE_FIELDS = "id, name, bio, skills, avatar_url, teach_subjects, learn_subjects, interests, is_mentor, is_learner, sessions_completed, rating, points, streak, badges, learning_style, preferred_language, timezone";
+
+// Fields available only to the profile owner (includes email, private info)
+const PRIVATE_PROFILE_FIELDS = PUBLIC_PROFILE_FIELDS + ", email, last_active, last_seen, availability, learning_goals, focus_time_this_week";
+
+// SECURITY (IDOR #1853): Server-side profile endpoint with authorization.
+// Replaces direct Supabase client calls that could be manipulated.
+// Returns public fields for other users, private fields for own profile.
+// NOTE: This endpoint intentionally does NOT use requireOwnershipOrAdmin
+// because any authenticated user should be able to view public profile data
+// (name, bio, skills, etc.). The field-level filtering below ensures that
+// private fields (email, last_active, etc.) are only returned to the owner.
+router.get("/:userId/profile", requireAuth, async (req, res) => {
+  try {
+    const { userId } = req.params;
+
+    // Strict UUID format validation to prevent injection
+    if (!userId || !UUID_REGEX.test(userId)) {
+      return res.status(400).json({ error: "Invalid user ID format" });
+    }
+
+    const supabaseAdmin = getSupabaseAdmin();
+    if (!supabaseAdmin) {
+      return res.status(500).json({ error: "Server configuration error" });
+    }
+
+    const isOwnProfile = req.user.id === userId;
+    const isAdmin = req.user?.role === "admin"
+      || req.user?.app_metadata?.role === "admin"
+      || req.roles?.includes("admin");
+
+    // Select fields based on who is requesting
+    const fields = (isOwnProfile || isAdmin) ? PRIVATE_PROFILE_FIELDS : PUBLIC_PROFILE_FIELDS;
+
+    const { data: profile, error } = await supabaseAdmin
+      .from("profiles")
+      .select(fields)
+      .eq("id", userId)
+      .maybeSingle();
+
+    if (error) {
+      console.error("[IDOR] Profile fetch error:", error.message);
+      return res.status(500).json({ error: "Failed to fetch profile" });
+    }
+
+    if (!profile) {
+      return res.status(404).json({ error: "Profile not found" });
+    }
+
+    return res.json({ success: true, profile });
+  } catch (err) {
+    console.error("[IDOR] Profile endpoint error:", err);
+    return res.status(500).json({ error: "Internal server error" });
+  }
+});
+
+// SECURITY (IDOR #1853): Server-side profile update with ownership enforcement.
+// Prevents users from modifying other users' profiles.
+router.put("/:userId/profile", requireAuth, requireOwnershipOrAdmin({ paramName: "userId" }), async (req, res) => {
+  try {
+    const { userId } = req.params;
+
+    if (!userId || !UUID_REGEX.test(userId)) {
+      return res.status(400).json({ error: "Invalid user ID format" });
+    }
+
+    const supabaseAdmin = getSupabaseAdmin();
+    if (!supabaseAdmin) {
+      return res.status(500).json({ error: "Server configuration error" });
+    }
+
+    // Only allow safe, non-gamification fields to be updated via this endpoint
+    const allowedFields = ["name", "bio", "skills", "avatar_url", "teach_subjects", "learn_subjects", "interests", "learning_style", "preferred_language", "timezone", "availability", "learning_goals"];
+    const updates = {};
+    for (const field of allowedFields) {
+      if (req.body[field] !== undefined) {
+        updates[field] = req.body[field];
+      }
+    }
+
+    if (Object.keys(updates).length === 0) {
+      return res.status(400).json({ error: "No valid fields to update" });
+    }
+
+    // Validate string lengths to prevent abuse
+    if (updates.name && typeof updates.name === "string" && updates.name.length > 100) {
+      return res.status(400).json({ error: "Name must be 100 characters or fewer" });
+    }
+    if (updates.bio && typeof updates.bio === "string" && updates.bio.length > 500) {
+      return res.status(400).json({ error: "Bio must be 500 characters or fewer" });
+    }
+
+    const { data: profile, error } = await supabaseAdmin
+      .from("profiles")
+      .update(updates)
+      .eq("id", userId)
+      .select("id, name, bio, skills, avatar_url")
+      .maybeSingle();
+
+    if (error) {
+      console.error("[IDOR] Profile update error:", error.message);
+      return res.status(500).json({ error: "Failed to update profile" });
+    }
+
+    return res.json({ success: true, profile });
+  } catch (err) {
+    console.error("[IDOR] Profile update endpoint error:", err);
+    return res.status(500).json({ error: "Internal server error" });
+  }
+});
+
 // User profile photo upload endpoint
 router.post("/upload-photo", requireAuth, uploadProfilePhoto, async (req, res) => {
   if (!req.file) {