Designing Patterns to Prevent IDOR
As a bug bounty triager and researcher, the most common type of bug I see is Insecure Direct Object Reference (IDOR). In their simplest form, IDOR bugs occur when a user provides an ID to the server that they do not normally have access to, and the server accepts it without validation.
For the purposes of this blog post, we’ll mostly be thinking about cross-tenant IDOR, where a user with membership to Organization A maliciously passes an ID belonging to Organization B to the backend.
IDOR bugs are prevalent for a few reasons:
- Input validation is usually data validation, and doesn’t consider external factors like the current logged in user
- Authorization code often asks the question “Can a user hit this route?”, not “Does the user have access to this ID?”
- Web frameworks don’t provide ways to completely mitigate IDOR, although Rails and other ORMs can help with querying data
- Developers, I suppose in general, don’t view users as malicious
- The adoption of UUIDs has made IDOR harder to mass exploit, as unique IDs aren’t guessable in the same way autoincrementing IDs are
Which all lead developers to treat each individual finding as its own failure - if there’s no generic solution to the problem already, it’s much easier to say “The bug is that this route didn’t validate user input” than “The bug is that IDOR is even remotely possible”.
What would these bug fixes look like if we attempted to make IDOR impossible? In this blog post we’ll explore architectural solutions that future-proof web applications from introducing IDOR, using the SevHunt codebase as a case study.
SevHunt uses TypeScript, tRPC, Zod, and Drizzle to serve backend routes from a Node server. The code samples are very TypeScript-y, but hopefully the concepts are applicable to your stack as well.
Here’s a made-up route we’ll use for an example:
// loggedInProcedure provides a tRPC route that makes sure the user is logged in
updateReportStatus: loggedInProcedure
.input(
// This is a Zod schema that validates user input
z.object({
organizationId: z.uuid(),
reportId: z.uuid(),
status: z.enum(["draft", "deleted", "new", "accepted", "fixed", "rejected"]),
}),
)
.mutation(async ({ ctx, input }) => {
// requireOrgMembershipValid ensures the user is a member of the given org
await requireOrgMembershipValid(ctx.user.id, input.organizationId);
// A Drizzle db query to update a report's status
await db.update(reports).set({
status: input.status
}).where(eq(reports.id, input.reportId))
return {};
}),
Can you spot the IDOR? While the logged in user’s (ctx.user.id’s) membership to the organization (input.organization) is validated, the report to update (input.reportId) is never verified to belong to that same organization.
Instead of manually fixing the route, let’s leave the contents of mutation(...) the same and think of some ideas how to make sure input.reportId is safe to consume.
Preamble - Sourcing the organization ID from the current resource
Before diving in, you may notice that the example route above could be secured loading the report and using its organizationId column to determine the “current” organization, instead of “deduplicating” this information in input.organizationId.
However, this approach only works for routes that perform actions on a single model/resource. If we consumed an array of report UUIDs to perform a bulk action instead, what would the “current” organization ID be? Would it be acceptable to perform a bulk operation on resources from different tenants in one request? There’s trade-offs when not explicitly providing an organization ID in every request path/payload for sure, but I wouldn’t view removing it as a solution for IDOR.
Idea 1 - Wrap raw values during validation
We use Zod to validate user input, for example to know an incoming string is a valid UUID, but we could also use it to transform input into arbitrary data types. For example, here is a Zod schema that returns a function that can be used with the current user ID to access the input organization ID:
export const zodWrappedOrganizationUuid = () => {
return z
.uuid()
.transform(async (organizationId) => {
// Zod returns a function instead of a value, which obfuscates the raw input from routes consuming it
return async (currentUserId: string): Promise<string> => {
await requireOrgMembershipValid(currentUserId, organizationId);
return organizationId;
};
});
};
It feels a little weird to mix authorization into validation, but the benefit is that the user-provided organization ID cannot be used without also validating the current user’s membership.
We can similarly add Zod schema that enforces that developers provide an organization ID before accessing organization-owned models:
// A minimal TypeScript type to ensure passed tables have these columns
type TableWithId = PgTable<TableConfig> & {
id: AnyPgColumn;
organizationId: AnyPgColumn;
};
export const zodWrappedModelUuid = <T extends TableWithId>(table: T) => {
return z
.uuid()
.transform(async (id) => {
// Return a function that requires a passed org ID, and returns the loaded model
return async (organizationId: string): Promise<InferSelectModel<T>> => {
const result = await db
.select()
.from(table as PgTable)
.where(and(
eq(table.id, id),
// Including an organization ID in the query implicitly prevents IDOR
eq(table.organizationId, organizationId),
));
if (result.length === 0) {
throw new TRPCError({ code: "NOT_FOUND" });
}
return result[0] as InferSelectModel<T>;
};
});
};
The types here are a bit more complex, but we’ve basically provided a way to stop developers from accessing incoming UUIDs directly, instead forcing them to provide a (hopefully validated) organization ID to load the actual model.
When used together in the route, the code looks like:
updateReportStatus: loggedInProcedure
.input(
z.object({
organizationId: zodWrappedOrganizationUuid(),
report: zodWrappedModelUuid(reports),
status: z.enum(["draft", "deleted", "new", "accepted", "fixed", "rejected"]),
}),
)
.mutation(async ({ ctx, input }) => {
// User input must be "unwrapped" before use
const organizationId = await input.organizationId(ctx.user.id)
const report = await input.report(organizationId)
// The query remains unchanged, but "report.id" is now safe to use
await db.update(reports).set({
status: input.status
}).where(eq(reports.id, report.id))
return {};
}),
While the implementation may be a little rocky, the core idea of making the raw IDs unusable is quite powerful. You’d also want to ban z.uuid() usage in your codebase, but that’s still better than trusting everyone to do validation properly by hand!
Returning objects instead of closures
It’d be nice if Zod constructed an actual type instead of returning a closure - here’s an example of how that could look with a class that uses TypeScript’s “private” keyword:
class WrappedModelId {
private unsafeRawId: string;
constructor(unsafeRawId: string) {
this.unsafeRawId = unsafeRawId;
}
public async unwrapModel<T extends TableWithId>(
table: T,
organizationId: string,
): Promise<InferSelectModel<T>> {
const result = await db
.select()
.from(table as PgTable)
.where(and(eq(table.id, this.unsafeRawId), eq(table.organizationId, organizationId)));
if (result.length === 0) {
throw new TRPCError({ code: "NOT_FOUND" });
}
return result[0] as InferSelectModel<T>;
}
}
export const zodModelId = () => {
return z
.uuid()
.transform((id) => new WrappedModelId(id));
};
Having the ID accessible without closures also enables you to provide library functions for bulk SELECTs and UPDATEs without performing N+1 "unwrap" queries.
Idea 2 - Add a tenant ID into every query
The idea behind wrapping is that we can't trust developers to consistently filter using a tenant ID. What if we let them write arbitrary queries, but ensured that those queries always started out "safe"?
There’s a good Drizzle discussion about this topic in GitHub, where a few users bring up wrapping Drizzle itself like so:
export const safeSelect = <T extends TableWithId>(
table: T,
organizationId: string,
...conditions: SQL[]
): PgSelect => {
return db
.select()
.from(table as PgTable)
.where(and(
eq(table.organizationId, organizationId),
...conditions
))
};
Which builds a Drizzle query for a given table that ensures an organization ID is always a part of the select. A consumer querying for reports may use it like so:
const result = await safeSelect(reports, input.organizationId, eq(reports.id, input.reportId));
I kind of like this, but wonder how it works with queries that use joins - should you also have helpers for that? What about INSERT/UPDATE, do we trust that safeSelect was already called or should we double check at every operation?
It’s a bit of a rabbit hole, but easier to parse than previous idea and relatively harmless to add to any existing Drizzle codebase.
An alternate Drizzle implementation
If you use Drizzle relations, which are kind of like Rails' Active Record Associations, you can push developers to build queries starting from the current organization, which can prevent many downstream IDOR bugs.
Idea 3 - Distinctly type your Primary Keys
In TypeScript there’s a clever way to introduce newtypes for data types like string - Branded Types. Here’s an example brand for making report IDs distinct from strings:
export type ReportId = string & { __brand: "ReportId" };
When we use this new type with our Drizzle schema, we can make the type system aware that random strings cannot be treated like “real” report IDs by using the $type() function:
import * as t from "drizzle-orm/pg-core";
export const reports = t.pgTable(
"reports",
{
id: t.uuid("id").primaryKey().$type<ReportId>(),
...
Now all queries for reports.id will fail type check if they pass in raw user input:
await db
.update(reports)
.set({
status: input.status,
})
// input.reportId is a string, and cannot type check without an assertion
.where(eq(reports.id, input.reportId));
This is a powerful path to introducing abstractions for proper handling of IDs, and at the very least might make a developer think twice before manually writing input.reportId as ReportId.
Of course, you'll also want to make sure developers don't add Zod schemas (JSON parsers) that transform strings to ReportId, but hopefully they're not that eager to circumvent your abstractions.
Addendum - Row Level Security
It feels wrong to write this blog post without mentioning Row Level Security (RLS), a SQL-native way of preventing IDOR. It truly deserves its own blog, but if you’re able to use RLS in your application you should consider it.
Without going into too much detail, RLS allows you to create policies that, among other things, append WHERE clauses to your queries to ensure that the tenant ID is checked.
I view RLS as a last line of defense for web applications - even if you fail to validate inputs it’ll keep your cross-tenant data safe, which is great. What isn’t great is all the uncertainty there - should you do these checks in both the application and database? What about IDOR within a tenant, should we replicate user permission checks in RLS? If our web app sees an INSUFFICIENT_PRIVILEGE error from Postgres, do we treat that as a bug or expected behavior?
Ultimately, how you use RLS is up to you, but in most cases you’re going to want something in the web application checking for IDOR too. Doing both is nice, doing one is fine, doing neither is probably the only wrong choice here.
Semi-related - Composite Primary Keys
I've also seen composite primary keys pitched as a solution for IDOR, where every table that has an organization_id has a primary key like PRIMARY KEY (id, organization_id). This ensures that foreign keys to those tables must be made on both IDs, which makes it less likely that you improperly point to a row from another tenant.
This is interesting, and a lot "lighter weight" than RLS, but doesn't address selects or joins (you can join on any column, even ones without indexes). That said, if you're already denormalizing organization_id in all your tables, it doesn't feel like that bad of an idea.
Conclusion
IDOR is a systemic problem, and as far as I know doesn’t have a turnkey "perfectly secure" solution in any modern web framework.
The three ideas above have a common theme - make insecure code feel worse to write than the secure defaults you provide. When developers write new web application routes they tend to copy existing patterns, not introduce new ones, and if your default pattern is to check for IDOR then you’re going to end up with less bugs.
Here at SevHunt we use shared authorization primitives that validate a user’s access to incoming IDs, but they aren’t foolproof - and while we’re not aware of IDOR in our codebase yet, we view the fact that it’s ever possible in the future as an open security vulnerability (which we're working on, obviously).
We encourage you to think of most bug bounty reports the same way - the problem isn’t that your developers did something wrong, it’s that they didn’t have a clear path for doing things right.