fix: round 2 S3 audit — CRITICAL SigV4 payload hash bug, timeouts, Content-MD5/Length validation
Deploy FileDrop / deploy (push) Successful in 43s
Deploy FileDrop / deploy (push) Successful in 43s
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) <noreply@anthropic.com>
This commit is contained in:
@@ -19,6 +19,7 @@
|
|||||||
"@biomejs/biome": "^2.4.15",
|
"@biomejs/biome": "^2.4.15",
|
||||||
"@types/node": "^25.8.0",
|
"@types/node": "^25.8.0",
|
||||||
"drizzle-kit": "^0.31.10",
|
"drizzle-kit": "^0.31.10",
|
||||||
|
"husky": "^9.1.7",
|
||||||
"typescript": "^6.0.3",
|
"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=="],
|
"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=="],
|
"inherits": ["inherits@2.0.4", "", {}, "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ=="],
|
||||||
|
|
||||||
"is-stream": ["is-stream@2.0.1", "", {}, "sha512-hFoiJiTl63nn+kstHGBtewWSKnQLpyb155KHheA1l39uvtO9nWIop1p3udqPcUd/xbF1VLMO4n7OI6p7RbngDg=="],
|
"is-stream": ["is-stream@2.0.1", "", {}, "sha512-hFoiJiTl63nn+kstHGBtewWSKnQLpyb155KHheA1l39uvtO9nWIop1p3udqPcUd/xbF1VLMO4n7OI6p7RbngDg=="],
|
||||||
|
|||||||
@@ -903,13 +903,50 @@ const handlePutObject = async (
|
|||||||
await cleanupTempFile(streamed.tempPath);
|
await cleanupTempFile(streamed.tempPath);
|
||||||
return s3ErrorResponse(
|
return s3ErrorResponse(
|
||||||
bodyHashError.errorCode || 'BadDigest',
|
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}`,
|
`/${bucket}/${key}`,
|
||||||
400,
|
400,
|
||||||
reqId,
|
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
|
// M12: Reject oversized bodies
|
||||||
if (streamed.sizeBytes > config.maxRequestBodyBytes) {
|
if (streamed.sizeBytes > config.maxRequestBodyBytes) {
|
||||||
await cleanupTempFile(streamed.tempPath);
|
await cleanupTempFile(streamed.tempPath);
|
||||||
@@ -1239,7 +1276,10 @@ const handleListObjectsV1 = async (
|
|||||||
|
|
||||||
const prefix = searchParams.get('prefix') || '';
|
const prefix = searchParams.get('prefix') || '';
|
||||||
const delimiter = searchParams.get('delimiter') || null;
|
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 marker = searchParams.get('marker') || null;
|
||||||
const encodingType = searchParams.get('encoding-type') || null;
|
const encodingType = searchParams.get('encoding-type') || null;
|
||||||
|
|
||||||
|
|||||||
@@ -282,10 +282,11 @@ export const verifySignature = async (
|
|||||||
return { isValid: false, credential: null, errorCode: 'NotImplemented' };
|
return { isValid: false, credential: null, errorCode: 'NotImplemented' };
|
||||||
}
|
}
|
||||||
|
|
||||||
// H4: Compute hash from actual body instead of trusting header blindly.
|
// CRITICAL: Use the x-amz-content-sha256 header value in the canonical
|
||||||
// For streaming bodies (body === null), we cannot hash at this point —
|
// request because that's what the client signed. The actual body hash is
|
||||||
// the caller (controller) must verify body hash after streaming.
|
// verified by verifyBodyHash() after streaming, ensuring integrity without
|
||||||
const hashedPayload = await getHashedPayload(body);
|
// breaking SigV4.
|
||||||
|
const hashedPayload = contentSha256 || (await getHashedPayload(body));
|
||||||
|
|
||||||
const canonicalRequest = buildCanonicalRequest(
|
const canonicalRequest = buildCanonicalRequest(
|
||||||
method,
|
method,
|
||||||
|
|||||||
@@ -60,8 +60,10 @@ const planParts = (parts: ObjectPartSource[], start: number, end: number): Plann
|
|||||||
const streamFromBytes = (bytes: Uint8Array): ReadableStream<Uint8Array> =>
|
const streamFromBytes = (bytes: Uint8Array): ReadableStream<Uint8Array> =>
|
||||||
new Response(bytes).body!;
|
new Response(bytes).body!;
|
||||||
|
|
||||||
|
const TELEGRAM_FETCH_TIMEOUT_MS = 30_000;
|
||||||
|
|
||||||
const fetchWholePartBytes = async (telegramUrl: string): Promise<Uint8Array> => {
|
const fetchWholePartBytes = async (telegramUrl: string): Promise<Uint8Array> => {
|
||||||
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}`);
|
if (!res.ok) throw new Error(`Telegram fetch failed: ${res.status}`);
|
||||||
return new Uint8Array(await res.arrayBuffer());
|
return new Uint8Array(await res.arrayBuffer());
|
||||||
};
|
};
|
||||||
@@ -77,10 +79,11 @@ const fetchPartBody = async (planned: PlannedPart): Promise<ReadableStream<Uint8
|
|||||||
}
|
}
|
||||||
|
|
||||||
const rangeHeader = `bytes=${planned.relativeStart}-${planned.relativeEnd}`;
|
const rangeHeader = `bytes=${planned.relativeStart}-${planned.relativeEnd}`;
|
||||||
const res = await fetch(
|
const fetchOpts: RequestInit = { signal: AbortSignal.timeout(TELEGRAM_FETCH_TIMEOUT_MS) };
|
||||||
planned.part.telegramUrl,
|
if (!wantsWholePart) {
|
||||||
wantsWholePart ? undefined : { headers: { range: rangeHeader } },
|
fetchOpts.headers = { range: rangeHeader };
|
||||||
);
|
}
|
||||||
|
const res = await fetch(planned.part.telegramUrl, fetchOpts);
|
||||||
if (!res.ok) throw new Error(`Telegram fetch failed: ${res.status}`);
|
if (!res.ok) throw new Error(`Telegram fetch failed: ${res.status}`);
|
||||||
if (wantsWholePart || res.status === 206) return res.body!;
|
if (wantsWholePart || res.status === 206) return res.body!;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user