From de7d2762455c4a4bc7908c60b9a3cbce5294acf3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 08:22:45 +0700 Subject: [PATCH] =?UTF-8?q?fix:=20round=202=20S3=20audit=20=E2=80=94=20CRI?= =?UTF-8?q?TICAL=20SigV4=20payload=20hash=20bug,=20timeouts,=20Content-MD5?= =?UTF-8?q?/Length=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL: - SigV4 canonical request used sha256Hex('') instead of x-amz-content-sha256 header value — every PUT/POST with body would fail 403. Now uses the signed header value for canonical request, verifyBodyHash after streaming for integrity. HIGH: - Add 30s AbortSignal.timeout to all Telegram CDN fetches in object-stream.ts (previously could hang indefinitely, exhausting connection pool) MEDIUM: - Content-MD5 validation: compute and compare when header is present - Content-Length validation: reject if actual body size != header - max-keys=0 clamping: enforce minimum of 1 per S3 spec Co-Authored-By: Claude Opus 5 (1M context) --- bun.lock | 3 ++ .../http/controllers/s3-controller.ts | 44 ++++++++++++++++++- src/utils/s3/auth.ts | 9 ++-- src/utils/s3/object-stream.ts | 13 +++--- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/bun.lock b/bun.lock index a986bd0..b16f734 100644 --- a/bun.lock +++ b/bun.lock @@ -19,6 +19,7 @@ "@biomejs/biome": "^2.4.15", "@types/node": "^25.8.0", "drizzle-kit": "^0.31.10", + "husky": "^9.1.7", "typescript": "^6.0.3", }, }, @@ -216,6 +217,8 @@ "get-tsconfig": ["get-tsconfig@4.14.0", "", { "dependencies": { "resolve-pkg-maps": "^1.0.0" } }, "sha512-yTb+8DXzDREzgvYmh6s9vHsSVCHeC0G3PI5bEXNBHtmshPnO+S5O7qgLEOn0I5QvMy6kpZN8K1NKGyilLb93wA=="], + "husky": ["husky@9.1.7", "", { "bin": { "husky": "bin.js" } }, "sha512-5gs5ytaNjBrh5Ow3zrvdUUY+0VxIuWVL4i9irt6friV+BqdCfmV11CQTWMiBYWHbXhco+J1kHfTOUkePhCDvMA=="], + "inherits": ["inherits@2.0.4", "", {}, "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ=="], "is-stream": ["is-stream@2.0.1", "", {}, "sha512-hFoiJiTl63nn+kstHGBtewWSKnQLpyb155KHheA1l39uvtO9nWIop1p3udqPcUd/xbF1VLMO4n7OI6p7RbngDg=="], diff --git a/src/interfaces/http/controllers/s3-controller.ts b/src/interfaces/http/controllers/s3-controller.ts index dd67415..b82134a 100644 --- a/src/interfaces/http/controllers/s3-controller.ts +++ b/src/interfaces/http/controllers/s3-controller.ts @@ -903,13 +903,50 @@ const handlePutObject = async ( await cleanupTempFile(streamed.tempPath); return s3ErrorResponse( bodyHashError.errorCode || 'BadDigest', - 'The Content-MD5 or x-amz-content-sha256 you specified did not match what we received.', + 'The x-amz-content-sha256 you specified did not match what we received.', `/${bucket}/${key}`, 400, reqId, ); } + // Content-Length validation: ensure actual body size matches header + const contentLengthHeader = headers['content-length']; + if (contentLengthHeader) { + const declaredLength = Number.parseInt(contentLengthHeader, 10); + if (Number.isFinite(declaredLength) && declaredLength !== streamed.sizeBytes) { + await cleanupTempFile(streamed.tempPath); + return s3ErrorResponse( + 'IncompleteBody', + 'You did not provide the number of bytes specified by the Content-Length HTTP header.', + `/${bucket}/${key}`, + 400, + reqId, + ); + } + } + + // Content-MD5 validation: verify MD5 when Content-MD5 header is present + const contentMd5 = headers['content-md5']; + if (contentMd5) { + const computedMd5 = Buffer.from( + await crypto.subtle.digest( + 'MD5', + new Uint8Array(await Bun.file(streamed.tempPath).arrayBuffer()), + ), + ).toString('base64'); + if (contentMd5 !== computedMd5) { + await cleanupTempFile(streamed.tempPath); + return s3ErrorResponse( + 'BadDigest', + 'The Content-MD5 you specified did not match what we received.', + `/${bucket}/${key}`, + 400, + reqId, + ); + } + } + // M12: Reject oversized bodies if (streamed.sizeBytes > config.maxRequestBodyBytes) { await cleanupTempFile(streamed.tempPath); @@ -1239,7 +1276,10 @@ const handleListObjectsV1 = async ( const prefix = searchParams.get('prefix') || ''; const delimiter = searchParams.get('delimiter') || null; - const maxKeys = Math.min(Number.parseInt(searchParams.get('max-keys') || '1000', 10), 1000); + const maxKeys = Math.max( + 1, + Math.min(Number.parseInt(searchParams.get('max-keys') || '1000', 10), 1000), + ); const marker = searchParams.get('marker') || null; const encodingType = searchParams.get('encoding-type') || null; diff --git a/src/utils/s3/auth.ts b/src/utils/s3/auth.ts index c693415..02a51e2 100644 --- a/src/utils/s3/auth.ts +++ b/src/utils/s3/auth.ts @@ -282,10 +282,11 @@ export const verifySignature = async ( return { isValid: false, credential: null, errorCode: 'NotImplemented' }; } - // H4: Compute hash from actual body instead of trusting header blindly. - // For streaming bodies (body === null), we cannot hash at this point — - // the caller (controller) must verify body hash after streaming. - const hashedPayload = await getHashedPayload(body); + // CRITICAL: Use the x-amz-content-sha256 header value in the canonical + // request because that's what the client signed. The actual body hash is + // verified by verifyBodyHash() after streaming, ensuring integrity without + // breaking SigV4. + const hashedPayload = contentSha256 || (await getHashedPayload(body)); const canonicalRequest = buildCanonicalRequest( method, diff --git a/src/utils/s3/object-stream.ts b/src/utils/s3/object-stream.ts index ca1e211..6d0e71f 100644 --- a/src/utils/s3/object-stream.ts +++ b/src/utils/s3/object-stream.ts @@ -60,8 +60,10 @@ const planParts = (parts: ObjectPartSource[], start: number, end: number): Plann const streamFromBytes = (bytes: Uint8Array): ReadableStream => new Response(bytes).body!; +const TELEGRAM_FETCH_TIMEOUT_MS = 30_000; + const fetchWholePartBytes = async (telegramUrl: string): Promise => { - const res = await fetch(telegramUrl); + const res = await fetch(telegramUrl, { signal: AbortSignal.timeout(TELEGRAM_FETCH_TIMEOUT_MS) }); if (!res.ok) throw new Error(`Telegram fetch failed: ${res.status}`); return new Uint8Array(await res.arrayBuffer()); }; @@ -77,10 +79,11 @@ const fetchPartBody = async (planned: PlannedPart): Promise