1
0

fix(server): use CSPRNG for attachment S3 keys (GHSA-gj48-p5ff-g67f)

The multer-s3 storage factory used `Date.now().toString()` as the S3 key
for every upload, yielding a 13-digit ms-epoch key. The keyspace for any
time window equals the millisecond count of that window, so an attacker
holding a registered account can enumerate keys for known upload moments
(e.g. ~10 minutes for a 10-second window with a 10-proxy rotation), then
download files via `GET /attachments/:id/presigned-url`. Two uploads in
the same millisecond also collide and silently overwrite each other.

Replace the key callback with `${organizationId}/${randomUUID()}`:

  * `randomUUID()` from `node:crypto` is a v4 UUID with 122 bits of
    entropy, making brute-force enumeration infeasible.
  * The `<organizationId>/` prefix (read from the `nestjs-cls` store
    populated by `ClsModule` middleware in `App.module.ts`) limits the
    blast radius of any hypothetical bucket-listing leak to a single
    tenant.

Add a tenant migration applying `unique` to `documents.key` so any future
key collision surfaces as a DB error instead of a silent S3 overwrite.

Legacy 13-digit numeric keys remain accessible via their stored values;
only new uploads use the new format.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Ahmed Bouhuolia
2026-05-16 19:38:39 +02:00
parent cd8d10afc0
commit ace15dbdeb
2 changed files with 24 additions and 3 deletions
@@ -0,0 +1,11 @@
exports.up = function (knex) {
return knex.schema.alterTable('documents', (table) => {
table.unique('key');
});
};
exports.down = function (knex) {
return knex.schema.alterTable('documents', (table) => {
table.dropUnique('key');
});
};
@@ -1,5 +1,7 @@
import { Module } from "@nestjs/common";
import { randomUUID } from 'node:crypto';
import * as multerS3 from 'multer-s3';
import { ClsService } from 'nestjs-cls';
import { S3_CLIENT, S3Module } from "../S3/S3.module";
import { DeleteAttachment } from "./DeleteAttachment";
import { GetAttachment } from "./GetAttachment";
@@ -59,8 +61,12 @@ const models = [
AttachmentUploadPipeline,
{
provide: MULTER_MODULE_OPTIONS,
inject: [ConfigService, S3_CLIENT],
useFactory: (configService: ConfigService, s3: S3Client) => ({
inject: [ConfigService, S3_CLIENT, ClsService],
useFactory: (
configService: ConfigService,
s3: S3Client,
cls: ClsService,
) => ({
storage: multerS3({
s3,
bucket: configService.get('s3.bucket'),
@@ -69,7 +75,11 @@ const models = [
cb(null, { fieldName: file.fieldname });
},
key: function (req, file, cb) {
cb(null, Date.now().toString());
const organizationId = cls.get<string>('organizationId');
if (!organizationId) {
return cb(new Error('Tenant context required for upload.'), undefined as any);
}
cb(null, `${organizationId}/${randomUUID()}`);
},
acl: function(req, file, cb) {
// Conditionally set file to public or private based on isPublic flag