Skip to content

Commit 3af16c6

Browse files
committed
fix: harden HTTP request security against downgrade and timing attacks
1 parent 5f1522c commit 3af16c6

2 files changed

Lines changed: 50 additions & 11 deletions

File tree

src/dlx/binary.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -566,13 +566,21 @@ export async function downloadBinaryFile(
566566
.digest('base64')
567567
const actualIntegrity = `sha512-${hash}`
568568

569-
// Verify integrity if provided.
570-
if (integrity && actualIntegrity !== integrity) {
571-
// Clean up invalid file.
572-
await safeDelete(destPath)
573-
throw new Error(
574-
`Integrity mismatch: expected ${integrity}, got ${actualIntegrity}`,
575-
)
569+
// Verify integrity if provided (constant-time comparison).
570+
if (integrity) {
571+
const integrityMatch =
572+
actualIntegrity.length === integrity.length &&
573+
crypto.timingSafeEqual(
574+
Buffer.from(actualIntegrity),
575+
Buffer.from(integrity),
576+
)
577+
if (!integrityMatch) {
578+
// Clean up invalid file.
579+
await safeDelete(destPath)
580+
throw new Error(
581+
`Integrity mismatch: expected ${integrity}, got ${actualIntegrity}`,
582+
)
583+
}
576584
}
577585

578586
// Make executable on POSIX systems.

src/http-request.ts

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,17 @@ async function httpDownloadAttempt(
776776
? res.headers.location
777777
: new URL(res.headers.location, url).toString()
778778

779+
// Reject HTTPS-to-HTTP downgrade redirects.
780+
const redirectParsed = new URL(redirectUrl)
781+
if (isHttps && redirectParsed.protocol !== 'https:') {
782+
reject(
783+
new Error(
784+
`Redirect from HTTPS to HTTP is not allowed: ${redirectUrl}`,
785+
),
786+
)
787+
return
788+
}
789+
779790
resolve(
780791
httpDownloadAttempt(redirectUrl, destPath, {
781792
ca,
@@ -946,6 +957,17 @@ async function httpRequestAttempt(
946957
? res.headers.location
947958
: new URL(res.headers.location, url).toString()
948959

960+
// Reject HTTPS-to-HTTP downgrade redirects.
961+
const redirectParsed = new URL(redirectUrl)
962+
if (isHttps && redirectParsed.protocol !== 'https:') {
963+
reject(
964+
new Error(
965+
`Redirect from HTTPS to HTTP is not allowed: ${redirectUrl}`,
966+
),
967+
)
968+
return
969+
}
970+
949971
resolve(
950972
httpRequestAttempt(redirectUrl, {
951973
body,
@@ -1142,7 +1164,9 @@ export async function httpDownload(
11421164
// This prevents partial/corrupted files at the destination path if download fails,
11431165
// and preserves the original file (if any) until download succeeds.
11441166
const fs = getFs()
1145-
const tempPath = `${destPath}.download`
1167+
const crypto = getCrypto()
1168+
const tempSuffix = crypto.randomBytes(6).toString('hex')
1169+
const tempPath = `${destPath}.${tempSuffix}.download`
11461170

11471171
// Clean up any stale temp file from a previous failed download.
11481172
if (fs.existsSync(tempPath)) {
@@ -1165,20 +1189,27 @@ export async function httpDownload(
11651189

11661190
// Verify checksum if sha256 hash is provided.
11671191
if (sha256) {
1168-
const crypto = getCrypto()
11691192
// eslint-disable-next-line no-await-in-loop
11701193
const fileContent = await fs.promises.readFile(tempPath)
11711194
const computedHash = crypto
11721195
.createHash('sha256')
11731196
.update(fileContent)
11741197
.digest('hex')
1198+
const expectedHash = sha256.toLowerCase()
11751199

1176-
if (computedHash !== sha256.toLowerCase()) {
1200+
// Use constant-time comparison to prevent timing attacks.
1201+
if (
1202+
computedHash.length !== expectedHash.length ||
1203+
!crypto.timingSafeEqual(
1204+
Buffer.from(computedHash),
1205+
Buffer.from(expectedHash),
1206+
)
1207+
) {
11771208
// eslint-disable-next-line no-await-in-loop
11781209
await safeDelete(tempPath)
11791210
throw new Error(
11801211
`Checksum verification failed for ${url}\n` +
1181-
`Expected: ${sha256.toLowerCase()}\n` +
1212+
`Expected: ${expectedHash}\n` +
11821213
`Computed: ${computedHash}`,
11831214
)
11841215
}

0 commit comments

Comments
 (0)