From beaab4da73e9b5d04820d06c6790bd1f2e00836c Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 17 Feb 2026 09:53:31 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20Fix=20IDOR?= =?UTF-8?q?=20vulnerabilities=20and=20enhance=20search=20security?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses several security issues in the chat and user actions: - Fixed IDOR vulnerabilities in `lib/actions/chat.ts` by ensuring all server actions (getChats, getChat, getChatMessages, saveChat, clearChats, updateDrawingContext, saveSystemPrompt, getSystemPrompt) use the authenticated session's userId instead of trusting client-provided arguments. - Added authorization checks to `getChatMessages` and `updateDrawingContext` to verify chat ownership/access. - Enhanced `searchUsers` in `lib/actions/users.ts` with input sanitization, escaping SQL wildcard characters (%, _, \) and enforcing a 255-character length limit to prevent wildcard enumeration and resource exhaustion. - Documented these security findings and preventions in `.jules/sentinel.md`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com> --- .jules/sentinel.md | 9 +++++ lib/actions/chat.ts | 94 +++++++++++++++++++++++++++++++++----------- lib/actions/users.ts | 9 ++++- 3 files changed, 89 insertions(+), 23 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..057638f8 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,9 @@ +## 2025-05-14 - [IDOR in Server Actions] +**Vulnerability:** Several server actions in `lib/actions/chat.ts` accepted a `userId` parameter from the client and used it directly for database queries without verification, allowing any authenticated user to read or modify data belonging to other users. +**Learning:** Next.js Server Actions are public API endpoints. Parameters passed from the client cannot be trusted for authorization. +**Prevention:** Always retrieve the `userId` from a secure session (e.g., via `getCurrentUserIdOnServer()`) inside the server action and ignore or verify any `userId` passed as an argument. + +## 2025-05-14 - [Wildcard Enumeration in SQL] +**Vulnerability:** The `searchUsers` function used unescaped user input in an `ilike` query, allowing users to enumerate the entire user table using wildcard characters like `%` or `_`. +**Learning:** Wildcard characters in `LIKE`/`ILIKE` queries can be abused to bypass filters or extract data if not properly escaped. +**Prevention:** Always escape `%`, `_`, and `\` in user input before using it in a `LIKE`/`ILIKE` query. Also, enforce length limits on search queries. diff --git a/lib/actions/chat.ts b/lib/actions/chat.ts index f36f2cf6..c3174a65 100644 --- a/lib/actions/chat.ts +++ b/lib/actions/chat.ts @@ -20,14 +20,19 @@ import { users } from '@/lib/db/schema' import { eq } from 'drizzle-orm' import { getCurrentUserIdOnServer } from '@/lib/auth/get-current-user' +/** + * Retrieves chats for the authenticated user. + * Protected against IDOR by using session-based userId. + */ export async function getChats(userId?: string | null): Promise { - if (!userId) { - console.warn('getChats called without userId, returning empty array.') - return [] + const currentUserId = await getCurrentUserIdOnServer(); + if (!currentUserId) { + console.warn('getChats: User not authenticated'); + return []; } try { - const { chats } = await dbGetChatsPage(userId, 20, 0) + const { chats } = await dbGetChatsPage(currentUserId, 20, 0) return chats } catch (error) { console.error('Error fetching chats from DB:', error) @@ -35,13 +40,19 @@ export async function getChats(userId?: string | null): Promise { } } +/** + * Retrieves a specific chat. + * Protected against IDOR by using session-based userId and verifying access. + */ export async function getChat(id: string, userId: string): Promise { - if (!userId) { - console.warn('getChat called without userId.') + const currentUserId = await getCurrentUserIdOnServer(); + if (!currentUserId) { + console.warn('getChat: User not authenticated'); return null; } + try { - const chat = await dbGetChat(id, userId) + const chat = await dbGetChat(id, currentUserId) return chat } catch (error) { console.error(`Error fetching chat ${id} from DB:`, error) @@ -51,13 +62,27 @@ export async function getChat(id: string, userId: string): Promise { if (!chatId) { console.warn('getChatMessages called without chatId'); return []; } + + const userId = await getCurrentUserIdOnServer(); + if (!userId) { + console.warn('getChatMessages: User not authenticated'); + return []; + } + try { + // Verify access (ownership or public visibility) + const chat = await dbGetChat(chatId, userId); + if (!chat) { + console.warn(`getChatMessages: User ${userId} does not have access to chat ${chatId}`); + return []; + } return dbGetMessagesByChatId(chatId); } catch (error) { console.error(`Error fetching messages for chat ${chatId} in getChatMessages:`, error); @@ -65,13 +90,17 @@ export async function getChatMessages(chatId: string): Promise } } +/** + * Clears all chats for the authenticated user. + * Protected against IDOR by using session-based userId. + */ export async function clearChats( - userId?: string | null + userId?: string | null // Kept for backward compatibility but ignored in favor of session userId ): Promise<{ error?: string } | void> { - const currentUserId = userId || (await getCurrentUserIdOnServer()) + const currentUserId = await getCurrentUserIdOnServer(); if (!currentUserId) { - console.error('clearChats: No user ID provided or found.') - return { error: 'User ID is required to clear chats' } + console.error('clearChats: User not authenticated.'); + return { error: 'Unauthorized: Not authenticated' } } try { @@ -87,16 +116,20 @@ export async function clearChats( } } +/** + * Saves a chat for the authenticated user. + * Protected against IDOR by using session-based userId. + */ export async function saveChat(chat: OldChatType, userId: string): Promise { - if (!userId && !chat.userId) { - console.error('saveChat: userId is required either as a parameter or in chat object.') + const currentUserId = await getCurrentUserIdOnServer(); + if (!currentUserId) { + console.error('saveChat: User not authenticated'); return null; } - const effectiveUserId = userId || chat.userId; const newChatData: DbNewChat = { id: chat.id, - userId: effectiveUserId, + userId: currentUserId, title: chat.title || 'Untitled Chat', createdAt: chat.createdAt ? new Date(chat.createdAt) : new Date(), visibility: 'private', @@ -105,7 +138,7 @@ export async function saveChat(chat: OldChatType, userId: string): Promise[] = chat.messages.map(msg => ({ id: msg.id, - userId: effectiveUserId, + userId: currentUserId, role: msg.role, content: typeof msg.content === 'object' ? JSON.stringify(msg.content) : msg.content, createdAt: msg.createdAt ? new Date(msg.createdAt) : new Date(), @@ -127,6 +160,13 @@ export async function updateDrawingContext(chatId: string, contextData: { drawnF return { error: 'User not authenticated' }; } + // Verify chat ownership before adding drawing context + const chat = await dbGetChat(chatId, userId); + if (!chat || chat.userId !== userId) { + console.error(`updateDrawingContext: User ${userId} does not own chat ${chatId}`); + return { error: 'Unauthorized: You do not have permission to update this chat' }; + } + const newDrawingMessage: DbNewMessage = { userId: userId, chatId: chatId, @@ -147,17 +187,22 @@ export async function updateDrawingContext(chatId: string, contextData: { drawnF } } +/** + * Saves the system prompt for the authenticated user. + * Uses session-based userId to prevent IDOR. + */ export async function saveSystemPrompt( - userId: string, + userId: string, // Kept for backward compatibility but ignored in favor of session userId prompt: string ): Promise<{ success?: boolean; error?: string }> { - if (!userId) return { error: 'User ID is required' } + const currentUserId = await getCurrentUserIdOnServer(); + if (!currentUserId) return { error: 'Unauthorized: Not authenticated' } if (!prompt) return { error: 'Prompt is required' } try { await db.update(users) .set({ systemPrompt: prompt }) - .where(eq(users.id, userId)); + .where(eq(users.id, currentUserId)); return { success: true } } catch (error) { @@ -166,15 +211,20 @@ export async function saveSystemPrompt( } } +/** + * Retrieves the system prompt for the authenticated user. + * Uses session-based userId to prevent IDOR. + */ export async function getSystemPrompt( - userId: string + userId: string // Kept for backward compatibility but ignored in favor of session userId ): Promise { - if (!userId) return null + const currentUserId = await getCurrentUserIdOnServer(); + if (!currentUserId) return null try { const result = await db.select({ systemPrompt: users.systemPrompt }) .from(users) - .where(eq(users.id, userId)) + .where(eq(users.id, currentUserId)) .limit(1); return result[0]?.systemPrompt || null; diff --git a/lib/actions/users.ts b/lib/actions/users.ts index 6fba7f8b..2d1ffe64 100644 --- a/lib/actions/users.ts +++ b/lib/actions/users.ts @@ -184,23 +184,30 @@ export async function saveSelectedModel(model: string): Promise<{ success: boole /** * Searches users by email. * Restricted to authenticated users. + * Sanitized to prevent wildcard enumeration. */ export async function searchUsers(query: string) { noStore(); if (!query) return []; + // Enforce input length limit to mitigate DoS/resource exhaustion + const sanitizedQuery = query.slice(0, 255); + const userId = await getCurrentUserIdOnServer(); if (!userId) { throw new Error('Unauthorized'); } try { + // Escape special characters (%, _, \) to prevent wildcard enumeration + const escapedQuery = sanitizedQuery.replace(/[%_\\]/g, '\\$&'); + const result = await db.select({ id: users.id, email: users.email, }) .from(users) - .where(ilike(users.email, `%${query}%`)) + .where(ilike(users.email, `%${escapedQuery}%`)) .limit(10); return result;