fix(security): file storage authorization hardening
Three separate issues in the file storage layer:
1. IDOR via client-controlled object key in applicant.saveFileMetadata
and file.replaceFile. Both procedures accepted `bucket` and `objectKey`
from the client and stored them on a new ProjectFile row attached to
the caller's own project. Because file.getDownloadUrl authorizes via
`findFirst({ bucket, objectKey })` -> projectId, an attacker could
bind another team's storage object to their own project row and then
download the foreign object through the legitimate authorization
path. Now both procedures require `bucket === BUCKET_NAME` and the
`objectKey` to start with the project's sanitized title prefix
(matches the prefix that generateObjectKey produces server-side).
New helper `objectKeyBelongsToProject` exported from src/lib/minio.ts;
`sanitizePath` is now exported as well so the helper can reuse it.
2. Missing per-round scope on file.getBulkDownloadUrls. The single-file
getDownloadUrl restricts a juror to files in rounds with sortOrder
<= their assigned round, but the bulk variant only checked that an
Assignment row existed for the project. A juror assigned only to
EVALUATION could pull URLs for LIVE_FINAL/DELIBERATION confidential
files via this endpoint. Now applies the same per-round filter when
the caller's access to the project is jury-only (mentors / team
members / award jurors retain unrestricted access, matching
getDownloadUrl semantics).
3. Same omission on the standalone /api/files/bulk-download REST route.
Same fix applied there.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -30,21 +30,21 @@ export async function POST(request: NextRequest): Promise<NextResponse> {
|
|||||||
// Authorization: must be admin or assigned jury/mentor for this project
|
// Authorization: must be admin or assigned jury/mentor for this project
|
||||||
const isAdmin = userRole === 'SUPER_ADMIN' || userRole === 'PROGRAM_ADMIN'
|
const isAdmin = userRole === 'SUPER_ADMIN' || userRole === 'PROGRAM_ADMIN'
|
||||||
|
|
||||||
|
// Per-round scope: jurors may only pull URLs for files in rounds with
|
||||||
|
// sortOrder <= their assigned round. Mirrors file.getDownloadUrl. Without
|
||||||
|
// this, a juror assigned to EVALUATION could bulk-download LIVE_FINAL
|
||||||
|
// confidential files via this endpoint.
|
||||||
|
let priorRoundIds: string[] | null = null
|
||||||
|
|
||||||
if (!isAdmin) {
|
if (!isAdmin) {
|
||||||
// Check if user is assigned as jury
|
|
||||||
const juryAssignment = await prisma.assignment.findFirst({
|
const juryAssignment = await prisma.assignment.findFirst({
|
||||||
where: {
|
where: { userId, projectId },
|
||||||
userId,
|
select: { id: true, roundId: true },
|
||||||
projectId,
|
|
||||||
},
|
|
||||||
})
|
})
|
||||||
|
|
||||||
// Check if user is assigned as mentor
|
|
||||||
const mentorAssignment = await prisma.mentorAssignment.findFirst({
|
const mentorAssignment = await prisma.mentorAssignment.findFirst({
|
||||||
where: {
|
where: { mentorId: userId, projectId },
|
||||||
mentorId: userId,
|
select: { id: true },
|
||||||
projectId,
|
|
||||||
},
|
|
||||||
})
|
})
|
||||||
|
|
||||||
if (!juryAssignment && !mentorAssignment) {
|
if (!juryAssignment && !mentorAssignment) {
|
||||||
@@ -53,14 +53,41 @@ export async function POST(request: NextRequest): Promise<NextResponse> {
|
|||||||
{ status: 403 }
|
{ status: 403 }
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Apply the per-round filter only when access is jury-only.
|
||||||
|
if (juryAssignment && !mentorAssignment) {
|
||||||
|
const assignedRound = await prisma.round.findUnique({
|
||||||
|
where: { id: juryAssignment.roundId },
|
||||||
|
select: { competitionId: true, sortOrder: true },
|
||||||
|
})
|
||||||
|
if (assignedRound) {
|
||||||
|
const priorOrCurrent = await prisma.round.findMany({
|
||||||
|
where: {
|
||||||
|
competitionId: assignedRound.competitionId,
|
||||||
|
sortOrder: { lte: assignedRound.sortOrder },
|
||||||
|
},
|
||||||
|
select: { id: true },
|
||||||
|
})
|
||||||
|
priorRoundIds = priorOrCurrent.map((r) => r.id)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fetch file metadata from DB
|
// Fetch file metadata from DB
|
||||||
|
const fileWhere: Record<string, unknown> = {
|
||||||
|
id: { in: fileIds },
|
||||||
|
projectId,
|
||||||
|
}
|
||||||
|
if (priorRoundIds !== null) {
|
||||||
|
fileWhere.OR = [
|
||||||
|
{ requirement: { roundId: { in: priorRoundIds } } },
|
||||||
|
{ requirementId: null, roundId: { in: priorRoundIds } },
|
||||||
|
{ requirementId: null, roundId: null },
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
const files = await prisma.projectFile.findMany({
|
const files = await prisma.projectFile.findMany({
|
||||||
where: {
|
where: fileWhere,
|
||||||
id: { in: fileIds },
|
|
||||||
projectId,
|
|
||||||
},
|
|
||||||
select: {
|
select: {
|
||||||
id: true,
|
id: true,
|
||||||
fileName: true,
|
fileName: true,
|
||||||
|
|||||||
@@ -116,7 +116,7 @@ export async function deleteObject(
|
|||||||
* Sanitize a name for use as a MinIO path segment.
|
* Sanitize a name for use as a MinIO path segment.
|
||||||
* Removes special characters, replaces spaces with underscores, limits length.
|
* Removes special characters, replaces spaces with underscores, limits length.
|
||||||
*/
|
*/
|
||||||
function sanitizePath(name: string): string {
|
export function sanitizePath(name: string): string {
|
||||||
return (
|
return (
|
||||||
name
|
name
|
||||||
.trim()
|
.trim()
|
||||||
@@ -164,3 +164,16 @@ export function generateMentorObjectKey(
|
|||||||
return generateObjectKey(projectTitle, fileName, 'mentorship')
|
return generateObjectKey(projectTitle, fileName, 'mentorship')
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Verify a client-supplied object key actually belongs to a project's
|
||||||
|
* sanitized prefix. Used to prevent cross-tenant binding where an attacker
|
||||||
|
* passes another team's `bucket+objectKey` into a metadata-save procedure.
|
||||||
|
*/
|
||||||
|
export function objectKeyBelongsToProject(
|
||||||
|
objectKey: string,
|
||||||
|
projectTitle: string,
|
||||||
|
): boolean {
|
||||||
|
const sanitized = sanitizePath(projectTitle)
|
||||||
|
return objectKey.startsWith(`${sanitized}/`)
|
||||||
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import crypto from 'crypto'
|
|||||||
import { z } from 'zod'
|
import { z } from 'zod'
|
||||||
import { TRPCError } from '@trpc/server'
|
import { TRPCError } from '@trpc/server'
|
||||||
import { router, publicProcedure, protectedProcedure } from '../trpc'
|
import { router, publicProcedure, protectedProcedure } from '../trpc'
|
||||||
import { getPresignedUrl, generateObjectKey, BUCKET_NAME } from '@/lib/minio'
|
import { getPresignedUrl, generateObjectKey, BUCKET_NAME, objectKeyBelongsToProject } from '@/lib/minio'
|
||||||
import { generateLogoKey, createStorageProvider, type StorageProviderType } from '@/lib/storage'
|
import { generateLogoKey, createStorageProvider, type StorageProviderType } from '@/lib/storage'
|
||||||
import { getImageUploadUrl, confirmImageUpload, getImageUrl, deleteImage, type ImageUploadConfig } from '@/server/utils/image-upload'
|
import { getImageUploadUrl, confirmImageUpload, getImageUrl, deleteImage, type ImageUploadConfig } from '@/server/utils/image-upload'
|
||||||
import { sendStyledNotificationEmail, sendTeamMemberInviteEmail } from '@/lib/email'
|
import { sendStyledNotificationEmail, sendTeamMemberInviteEmail } from '@/lib/email'
|
||||||
@@ -408,6 +408,24 @@ export const applicantRouter = router({
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Verify the client-supplied objectKey actually belongs to this project's
|
||||||
|
// sanitized prefix. Without this, a user could pass another team's
|
||||||
|
// bucket+objectKey here, attach it to their own project row, and then
|
||||||
|
// download the foreign object via file.getDownloadUrl (which authorizes
|
||||||
|
// by bucket+objectKey -> projectId).
|
||||||
|
if (input.bucket !== BUCKET_NAME) {
|
||||||
|
throw new TRPCError({
|
||||||
|
code: 'BAD_REQUEST',
|
||||||
|
message: 'Invalid file location',
|
||||||
|
})
|
||||||
|
}
|
||||||
|
if (!objectKeyBelongsToProject(input.objectKey, project.title)) {
|
||||||
|
throw new TRPCError({
|
||||||
|
code: 'BAD_REQUEST',
|
||||||
|
message: 'Invalid file location for this project',
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// Block rejected projects
|
// Block rejected projects
|
||||||
if (await isProjectRejected(ctx.prisma, input.projectId)) {
|
if (await isProjectRejected(ctx.prisma, input.projectId)) {
|
||||||
throw new TRPCError({ code: 'FORBIDDEN', message: 'Your project has been rejected. File changes are no longer permitted.' })
|
throw new TRPCError({ code: 'FORBIDDEN', message: 'Your project has been rejected. File changes are no longer permitted.' })
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { z } from 'zod'
|
import { z } from 'zod'
|
||||||
import { TRPCError } from '@trpc/server'
|
import { TRPCError } from '@trpc/server'
|
||||||
import { router, protectedProcedure, adminProcedure } from '../trpc'
|
import { router, protectedProcedure, adminProcedure } from '../trpc'
|
||||||
import { getPresignedUrl, generateObjectKey, deleteObject, BUCKET_NAME } from '@/lib/minio'
|
import { getPresignedUrl, generateObjectKey, deleteObject, BUCKET_NAME, objectKeyBelongsToProject } from '@/lib/minio'
|
||||||
import { logAudit } from '../utils/audit'
|
import { logAudit } from '../utils/audit'
|
||||||
import { checkRequirementsAndTransition } from '../services/round-engine'
|
import { checkRequirementsAndTransition } from '../services/round-engine'
|
||||||
|
|
||||||
@@ -556,6 +556,28 @@ export const fileRouter = router({
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Verify the client-supplied objectKey actually belongs to this project's
|
||||||
|
// sanitized prefix. Without this check, a team member (or admin) could
|
||||||
|
// pass another project's bucket+objectKey here and bind it as a "new
|
||||||
|
// version" — then anyone with project access could read the foreign
|
||||||
|
// object via file.getDownloadUrl, which authorizes by bucket+objectKey.
|
||||||
|
const project = await ctx.prisma.project.findUniqueOrThrow({
|
||||||
|
where: { id: input.projectId },
|
||||||
|
select: { title: true },
|
||||||
|
})
|
||||||
|
if (input.bucket !== BUCKET_NAME) {
|
||||||
|
throw new TRPCError({
|
||||||
|
code: 'BAD_REQUEST',
|
||||||
|
message: 'Invalid file location',
|
||||||
|
})
|
||||||
|
}
|
||||||
|
if (!objectKeyBelongsToProject(input.objectKey, project.title)) {
|
||||||
|
throw new TRPCError({
|
||||||
|
code: 'BAD_REQUEST',
|
||||||
|
message: 'Invalid file location for this project',
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// Create new file and update old file in a transaction
|
// Create new file and update old file in a transaction
|
||||||
const result = await ctx.prisma.$transaction(async (tx) => {
|
const result = await ctx.prisma.$transaction(async (tx) => {
|
||||||
const newFile = await tx.projectFile.create({
|
const newFile = await tx.projectFile.create({
|
||||||
@@ -696,11 +718,18 @@ export const fileRouter = router({
|
|||||||
.query(async ({ ctx, input }) => {
|
.query(async ({ ctx, input }) => {
|
||||||
const isAdminOrObserver = ['SUPER_ADMIN', 'PROGRAM_ADMIN', 'OBSERVER', 'AWARD_MASTER'].includes(ctx.user.role)
|
const isAdminOrObserver = ['SUPER_ADMIN', 'PROGRAM_ADMIN', 'OBSERVER', 'AWARD_MASTER'].includes(ctx.user.role)
|
||||||
|
|
||||||
|
// For non-admin/observer callers, mirror the per-round scope used by
|
||||||
|
// file.getDownloadUrl: a juror assigned to round N may only pull URLs
|
||||||
|
// for files in rounds with sortOrder <= N. Without this, a juror
|
||||||
|
// assigned to EVALUATION can bulk-download LIVE_FINAL/DELIBERATION
|
||||||
|
// confidential files via this endpoint.
|
||||||
|
let priorRoundIds: string[] | null = null
|
||||||
|
|
||||||
if (!isAdminOrObserver) {
|
if (!isAdminOrObserver) {
|
||||||
const [assignment, mentorAssignment, teamMembership] = await Promise.all([
|
const [assignment, mentorAssignment, teamMembership] = await Promise.all([
|
||||||
ctx.prisma.assignment.findFirst({
|
ctx.prisma.assignment.findFirst({
|
||||||
where: { userId: ctx.user.id, projectId: input.projectId },
|
where: { userId: ctx.user.id, projectId: input.projectId },
|
||||||
select: { id: true },
|
select: { id: true, roundId: true },
|
||||||
}),
|
}),
|
||||||
ctx.prisma.mentorAssignment.findFirst({
|
ctx.prisma.mentorAssignment.findFirst({
|
||||||
where: { mentorId: ctx.user.id, projectId: input.projectId },
|
where: { mentorId: ctx.user.id, projectId: input.projectId },
|
||||||
@@ -724,6 +753,26 @@ export const fileRouter = router({
|
|||||||
message: 'You do not have access to this project\'s files',
|
message: 'You do not have access to this project\'s files',
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Apply per-round filter only when the caller's access is jury-only.
|
||||||
|
// Mentors / team members / award jurors get unrestricted file access
|
||||||
|
// on the project (matches getDownloadUrl semantics).
|
||||||
|
if (assignment && !mentorAssignment && !teamMembership) {
|
||||||
|
const assignedRound = await ctx.prisma.round.findUnique({
|
||||||
|
where: { id: assignment.roundId },
|
||||||
|
select: { competitionId: true, sortOrder: true },
|
||||||
|
})
|
||||||
|
if (assignedRound) {
|
||||||
|
const priorOrCurrent = await ctx.prisma.round.findMany({
|
||||||
|
where: {
|
||||||
|
competitionId: assignedRound.competitionId,
|
||||||
|
sortOrder: { lte: assignedRound.sortOrder },
|
||||||
|
},
|
||||||
|
select: { id: true },
|
||||||
|
})
|
||||||
|
priorRoundIds = priorOrCurrent.map((r) => r.id)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get files
|
// Get files
|
||||||
@@ -731,6 +780,17 @@ export const fileRouter = router({
|
|||||||
if (input.fileIds && input.fileIds.length > 0) {
|
if (input.fileIds && input.fileIds.length > 0) {
|
||||||
where.id = { in: input.fileIds }
|
where.id = { in: input.fileIds }
|
||||||
}
|
}
|
||||||
|
if (priorRoundIds !== null) {
|
||||||
|
// Allow a file when:
|
||||||
|
// - it has a requirement whose round is in the allowed set, OR
|
||||||
|
// - it has no requirement and its roundId is in the allowed set, OR
|
||||||
|
// - it has neither requirement nor roundId (legacy general files)
|
||||||
|
where.OR = [
|
||||||
|
{ requirement: { roundId: { in: priorRoundIds } } },
|
||||||
|
{ requirementId: null, roundId: { in: priorRoundIds } },
|
||||||
|
{ requirementId: null, roundId: null },
|
||||||
|
]
|
||||||
|
}
|
||||||
|
|
||||||
const files = await ctx.prisma.projectFile.findMany({
|
const files = await ctx.prisma.projectFile.findMany({
|
||||||
where,
|
where,
|
||||||
|
|||||||
Reference in New Issue
Block a user