_⚠️ Potential issue_ | _🟠 Major_ #6
Description
TOCTOU vulnerability: race condition between uniqueness checks and insert.
The uniqueness checks (lines 56-67 and 70-81) and the insert (line 85) are not atomic. Two concurrent requests with the same username or email could both pass the checks before either inserts. The database's unique constraint on username will catch one, but there's no such constraint on email (as noted in the migration review).
Additionally, consider combining the two uniqueness checks into a single query for efficiency.
🔎 Recommended approach
- Add a unique constraint on
emailin the database schema (primary fix) - Optionally use a transaction or handle the unique constraint violation gracefully:
try {
await db.insert(accessRequest).values({ ... })
} catch (error) {
// Check if it's a unique constraint violation
if (error.code === '23505') { // PostgreSQL unique violation
return new Response(
JSON.stringify({ error: 'An access request with this username or email already exists' }),
{ status: 409, headers: { 'Content-Type': 'application/json' } },
)
}
throw error
}- Combine uniqueness checks:
import { or } from 'drizzle-orm'
const existing = await db
.select({ username: accessRequest.username, email: accessRequest.email })
.from(accessRequest)
.where(or(
eq(accessRequest.username, username.toLowerCase()),
eq(accessRequest.email, email.toLowerCase())
))
.limit(1)
if (existing.length > 0) {
const field = existing[0].username === username.toLowerCase() ? 'username' : 'email'
return new Response(
JSON.stringify({ error: `An access request with this ${field} already exists` }),
{ status: 409, headers: { 'Content-Type': 'application/json' } },
)
}Originally posted by @coderabbitai[bot] in #1 (comment)