Security Concern: Client-Controlled userId Leads to Potential Privilege Escalation in Server Actions & Queries

[CuteNikki]

Hi Theo,

Thanks for the amazing tutorial! While exploring the project, I noticed a potential security issue regarding how userId is passed to server actions and DB queries.

Problem:

In src/app/(home)/drive/page.tsx, the userId is retrieved and passed into server actions like getRootFolderForUser and onboardUser. This value is then passed directly into queries in src/server/db/queries.ts. Since userId is controlled by the client, this introduces a horizontal privilege escalation risk.

Potential Security Risk:

By accepting userId directly from the client, there's a risk that malicious users could manipulate this value to access or modify data belonging to other users. This could lead to unauthorized actions, such as viewing or creating another user's files.

Suggested Fix:

Resolve userId exclusively server-side.
Only pass fileId or folderId from the client.

Within each function that requires a userId, obtain it directly from the server-side authentication context. For instance, using Clerk's server-side SDK:

Before:

export const MUTATIONS = {
  // ...
  createFile: async function (input: {
    file: {
      name: string;
      size: number;
      url: string;
      parent: number;
    };
    userId: string;
  }) {
    return await db.insert(filesSchema).values({
      ...input.file,
      ownerId: input.userId,
    });
  },
  // ...
};

After:

import { auth } from "@clerk/nextjs/server";

export const MUTATIONS = {
  // ...
  createFile: async function (file: {
      name: string;
      size: number;
      url: string;
      parent: number;
    }) {
    const { userId } = auth();
    if (!userId) {
      throw new Error("Unauthorized"); // or navigate the user?
    }

    return await db.insert(filesSchema).values({
      ...file,
      ownerId: userId,
    });
  },
  // ...
  // Similarly update other functions
};

Why it matters

Server actions and DB queries could be exploited by tampered client inputs.
A malicious user could craft a request with a different userId and potentially create, delete or modify files that belong to other users.
Resolving userId server-side + strict ownership checks will mitigate horizontal privilege escalation risks.

Thanks again for the great content! Your efforts in creating educational content are greatly appreciated!

[EricTsai83]

In the file src/app/(home)/drive/page.tsx, we use Clerk's auth function to obtain the server-side session. Since this code executes on the server, the session.userId is retrieved only after proper authentication and is not arbitrarily supplied by the client.

I am confident that adding authentication to each function in actions.ts is best practice, but I'm not sure whether it's necessary to add authentication to every database connection function.

I’d appreciate any insights or recommendations. Thanks!