Verify update installer SHA-256; harden download teardown
Integrity + defence-in-depth for the in-app updater: - Require and verify a SHA-256 checksum before running an installer. Each release must attach `<installer>.sha256`; downloadAppUpdate fetches it from the host-pinned origin (deriving the URL itself, not trusting the renderer), hashes the stream as it writes, and refuses to run on mismatch/missing/ unparseable. Fails closed. NB: same-host hash is integrity, not protection against a fully compromised host — only Authenticode signing covers that. - fetchTrustedText helper GETs the checksum with the same per-hop host re-validation as the installer download, plus a size cap and timeout. - finish() is now the single teardown point and aborts the request on failure, so a write error can't leave Electron pulling bytes into a dead stream; removed three now-redundant explicit abort() calls. - Clear the idle/stall timer once the body is fully received. - Export isTrustedDownloadUrl/compareVersions, extract extractSha256, and add 14 unit tests (host pin incl. userinfo spoof + wrong port, checksum parsing incl. sha512 non-slice, prerelease never outranking its release). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+115
-5
@@ -2,6 +2,7 @@ import { app, net, shell, type WebContents } from 'electron'
|
|||||||
import { createWriteStream, type WriteStream } from 'fs'
|
import { createWriteStream, type WriteStream } from 'fs'
|
||||||
import { stat, unlink } from 'fs/promises'
|
import { stat, unlink } from 'fs/promises'
|
||||||
import { join, normalize, dirname } from 'path'
|
import { join, normalize, dirname } from 'path'
|
||||||
|
import { createHash } from 'crypto'
|
||||||
import {
|
import {
|
||||||
IpcChannels,
|
IpcChannels,
|
||||||
type AppUpdateInfo,
|
type AppUpdateInfo,
|
||||||
@@ -28,7 +29,7 @@ const RELEASE_API = `${UPDATE_HOST}/api/v1/repos/${UPDATE_OWNER}/${UPDATE_REPO}/
|
|||||||
// release downloads to its attachment store — so this is re-checked on EVERY
|
// release downloads to its attachment store — so this is re-checked on EVERY
|
||||||
// redirect hop (see downloadAppUpdate), which is what stops a tampered/MITM'd
|
// redirect hop (see downloadAppUpdate), which is what stops a tampered/MITM'd
|
||||||
// response from bouncing us off the trusted host.
|
// response from bouncing us off the trusted host.
|
||||||
function isTrustedDownloadUrl(url: string): boolean {
|
export function isTrustedDownloadUrl(url: string): boolean {
|
||||||
try {
|
try {
|
||||||
const u = new URL(url)
|
const u = new URL(url)
|
||||||
return u.protocol === 'https:' && u.host === new URL(UPDATE_HOST).host
|
return u.protocol === 'https:' && u.host === new URL(UPDATE_HOST).host
|
||||||
@@ -59,7 +60,7 @@ function parseVersion(v: string): number[] {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Component-wise numeric compare: -1 if a<b, 0 if equal, 1 if a>b. */
|
/** Component-wise numeric compare: -1 if a<b, 0 if equal, 1 if a>b. */
|
||||||
function compareVersions(a: string, b: string): number {
|
export function compareVersions(a: string, b: string): number {
|
||||||
const pa = parseVersion(a)
|
const pa = parseVersion(a)
|
||||||
const pb = parseVersion(b)
|
const pb = parseVersion(b)
|
||||||
const len = Math.max(pa.length, pb.length)
|
const len = Math.max(pa.length, pb.length)
|
||||||
@@ -82,6 +83,17 @@ interface GiteaRelease {
|
|||||||
assets?: GiteaAsset[]
|
assets?: GiteaAsset[]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Pull the SHA-256 hex out of a checksum file — a bare digest, `sha256sum`
|
||||||
|
* output (`<hash> file`), or PowerShell Get-FileHash (uppercase). Returns the
|
||||||
|
* lowercase digest, or null if there's no standalone 64-char hex token (so a
|
||||||
|
* longer run like a sha512 digest is ignored rather than sliced).
|
||||||
|
*/
|
||||||
|
export function extractSha256(text: string): string | null {
|
||||||
|
const m = text.match(/\b[a-f0-9]{64}\b/i)
|
||||||
|
return m ? m[0].toLowerCase() : null
|
||||||
|
}
|
||||||
|
|
||||||
/** Pick the Windows installer asset — prefer the NSIS "Setup" .exe, else any .exe. */
|
/** Pick the Windows installer asset — prefer the NSIS "Setup" .exe, else any .exe. */
|
||||||
function pickInstaller(assets: GiteaAsset[]): GiteaAsset | undefined {
|
function pickInstaller(assets: GiteaAsset[]): GiteaAsset | undefined {
|
||||||
const exes = assets.filter((a) => /\.exe$/i.test(a.name))
|
const exes = assets.filter((a) => /\.exe$/i.test(a.name))
|
||||||
@@ -146,6 +158,65 @@ export async function checkForAppUpdate(): Promise<AppUpdateInfo> {
|
|||||||
/** How long the download may stall (no bytes received) before we give up. */
|
/** How long the download may stall (no bytes received) before we give up. */
|
||||||
const DOWNLOAD_IDLE_TIMEOUT_MS = 60_000
|
const DOWNLOAD_IDLE_TIMEOUT_MS = 60_000
|
||||||
|
|
||||||
|
// Integrity: every release MUST attach a checksum asset named `<installer>.sha256`
|
||||||
|
// (e.g. AeroFetch-Setup-0.5.0.exe.sha256) whose contents include the installer's
|
||||||
|
// lowercase SHA-256 hex — bare, or in `sha256sum`/`Get-FileHash` form. We refuse
|
||||||
|
// to run an installer we can't verify against it. Note this is INTEGRITY +
|
||||||
|
// defence-in-depth (corruption, truncation, tampering-after-publish, a redirect
|
||||||
|
// bounced to other storage), NOT protection against a fully compromised host —
|
||||||
|
// that host serves both files, so only Authenticode signing defends against it.
|
||||||
|
const REQUIRE_CHECKSUM: boolean = true
|
||||||
|
|
||||||
|
/**
|
||||||
|
* GET a small text resource from the trusted host, re-validating the host on
|
||||||
|
* every redirect hop (same discipline as the installer download) and capping the
|
||||||
|
* body so a hostile/huge response can't exhaust memory. Used for the .sha256 file.
|
||||||
|
*/
|
||||||
|
function fetchTrustedText(
|
||||||
|
url: string,
|
||||||
|
maxBytes = 64 * 1024,
|
||||||
|
timeoutMs = 15_000
|
||||||
|
): Promise<{ ok: true; text: string } | { ok: false; status?: number; error: string }> {
|
||||||
|
return new Promise((resolve) => {
|
||||||
|
let settled = false
|
||||||
|
const done = (r: { ok: true; text: string } | { ok: false; status?: number; error: string }): void => {
|
||||||
|
if (settled) return
|
||||||
|
settled = true
|
||||||
|
clearTimeout(timer)
|
||||||
|
if (!r.ok) request.abort()
|
||||||
|
resolve(r)
|
||||||
|
}
|
||||||
|
const request = net.request({ url, redirect: 'manual' })
|
||||||
|
const timer = setTimeout(() => done({ ok: false, error: 'timed out' }), timeoutMs)
|
||||||
|
request.on('redirect', (_s, _m, redirectUrl) => {
|
||||||
|
if (!isTrustedDownloadUrl(redirectUrl)) {
|
||||||
|
done({ ok: false, error: 'redirect to an untrusted location' })
|
||||||
|
return
|
||||||
|
}
|
||||||
|
request.followRedirect()
|
||||||
|
})
|
||||||
|
request.on('response', (response) => {
|
||||||
|
const status = response.statusCode
|
||||||
|
if (status < 200 || status >= 300) {
|
||||||
|
done({ ok: false, status, error: `HTTP ${status}` })
|
||||||
|
return
|
||||||
|
}
|
||||||
|
const chunks: Buffer[] = []
|
||||||
|
let size = 0
|
||||||
|
response.on('data', (c: Buffer) => {
|
||||||
|
size += c.length
|
||||||
|
if (size > maxBytes) done({ ok: false, error: 'checksum file too large' })
|
||||||
|
else chunks.push(c)
|
||||||
|
})
|
||||||
|
response.on('end', () => done({ ok: true, text: Buffer.concat(chunks).toString('utf8') }))
|
||||||
|
response.on('aborted', () => done({ ok: false, error: 'interrupted' }))
|
||||||
|
response.on('error', (e: Error) => done({ ok: false, error: e.message }))
|
||||||
|
})
|
||||||
|
request.on('error', (e: Error) => done({ ok: false, error: e.message }))
|
||||||
|
request.end()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
/** Stream the installer to a temp file, pushing progress events to the renderer. */
|
/** Stream the installer to a temp file, pushing progress events to the renderer. */
|
||||||
export async function downloadAppUpdate(url: string, wc: WebContents): Promise<AppUpdateDownload> {
|
export async function downloadAppUpdate(url: string, wc: WebContents): Promise<AppUpdateDownload> {
|
||||||
if (!isTrustedDownloadUrl(url)) {
|
if (!isTrustedDownloadUrl(url)) {
|
||||||
@@ -157,16 +228,44 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise<A
|
|||||||
const safeName = /^[\w.\- ]+\.exe$/i.test(urlName) ? urlName : 'AeroFetch-Setup.exe'
|
const safeName = /^[\w.\- ]+\.exe$/i.test(urlName) ? urlName : 'AeroFetch-Setup.exe'
|
||||||
const filePath = join(updateDir(), safeName)
|
const filePath = join(updateDir(), safeName)
|
||||||
|
|
||||||
|
// Resolve the published checksum up front (tiny, fails fast) so we never pull a
|
||||||
|
// ~300 MB installer we'd then refuse to run. The .sha256 sits next to the asset,
|
||||||
|
// so its URL is the installer URL + '.sha256' — still on the host-pinned origin.
|
||||||
|
const checksumUrl = (() => {
|
||||||
|
const u = new URL(url)
|
||||||
|
u.pathname += '.sha256'
|
||||||
|
return u.toString()
|
||||||
|
})()
|
||||||
|
const sum = await fetchTrustedText(checksumUrl)
|
||||||
|
let expectedSha: string | null = null
|
||||||
|
if (sum.ok) {
|
||||||
|
expectedSha = extractSha256(sum.text)
|
||||||
|
if (!expectedSha) return { ok: false, error: "The update's checksum file is malformed." }
|
||||||
|
} else if (sum.status === 404) {
|
||||||
|
if (REQUIRE_CHECKSUM) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
error: `This release has no checksum (${safeName}.sha256) attached — refusing to install an unverified update.`
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
return { ok: false, error: `Couldn't fetch the update's checksum — ${sum.error}.` }
|
||||||
|
}
|
||||||
|
|
||||||
return new Promise<AppUpdateDownload>((resolve) => {
|
return new Promise<AppUpdateDownload>((resolve) => {
|
||||||
let settled = false
|
let settled = false
|
||||||
let fileStream: WriteStream | null = null
|
let fileStream: WriteStream | null = null
|
||||||
let idle: NodeJS.Timeout | null = null
|
let idle: NodeJS.Timeout | null = null
|
||||||
|
|
||||||
|
// Single teardown point. On failure it also aborts the request, so a write
|
||||||
|
// error (disk full, etc.) can't leave Electron pulling bytes into a dead
|
||||||
|
// stream. abort() is idempotent, so the call sites below don't repeat it.
|
||||||
const finish = (result: AppUpdateDownload): void => {
|
const finish = (result: AppUpdateDownload): void => {
|
||||||
if (settled) return
|
if (settled) return
|
||||||
settled = true
|
settled = true
|
||||||
if (idle) clearTimeout(idle)
|
if (idle) clearTimeout(idle)
|
||||||
if (!result.ok) {
|
if (!result.ok) {
|
||||||
|
request.abort()
|
||||||
// Close the handle before unlinking (Windows won't delete an open file)
|
// Close the handle before unlinking (Windows won't delete an open file)
|
||||||
// and never leave a partial/aborted installer lying around in temp.
|
// and never leave a partial/aborted installer lying around in temp.
|
||||||
if (fileStream && !fileStream.destroyed) fileStream.destroy()
|
if (fileStream && !fileStream.destroyed) fileStream.destroy()
|
||||||
@@ -184,14 +283,12 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise<A
|
|||||||
const armIdle = (): void => {
|
const armIdle = (): void => {
|
||||||
if (idle) clearTimeout(idle)
|
if (idle) clearTimeout(idle)
|
||||||
idle = setTimeout(() => {
|
idle = setTimeout(() => {
|
||||||
request.abort()
|
|
||||||
finish({ ok: false, error: 'Download stalled — please try again.' })
|
finish({ ok: false, error: 'Download stalled — please try again.' })
|
||||||
}, DOWNLOAD_IDLE_TIMEOUT_MS)
|
}, DOWNLOAD_IDLE_TIMEOUT_MS)
|
||||||
}
|
}
|
||||||
|
|
||||||
request.on('redirect', (_status, _method, redirectUrl) => {
|
request.on('redirect', (_status, _method, redirectUrl) => {
|
||||||
if (!isTrustedDownloadUrl(redirectUrl)) {
|
if (!isTrustedDownloadUrl(redirectUrl)) {
|
||||||
request.abort()
|
|
||||||
finish({ ok: false, error: 'Refused to follow a redirect to an untrusted location.' })
|
finish({ ok: false, error: 'Refused to follow a redirect to an untrusted location.' })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -201,7 +298,6 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise<A
|
|||||||
request.on('response', (response) => {
|
request.on('response', (response) => {
|
||||||
const status = response.statusCode
|
const status = response.statusCode
|
||||||
if (status < 200 || status >= 300) {
|
if (status < 200 || status >= 300) {
|
||||||
request.abort()
|
|
||||||
finish({ ok: false, error: `Download failed (HTTP ${status}).` })
|
finish({ ok: false, error: `Download failed (HTTP ${status}).` })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -211,6 +307,8 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise<A
|
|||||||
|
|
||||||
const stream = createWriteStream(filePath)
|
const stream = createWriteStream(filePath)
|
||||||
fileStream = stream
|
fileStream = stream
|
||||||
|
// Hash the bytes as they stream by, so verification needs no second pass.
|
||||||
|
const hash = createHash('sha256')
|
||||||
// Electron's IncomingMessage is event-based (no .pipe). pause()/resume()
|
// Electron's IncomingMessage is event-based (no .pipe). pause()/resume()
|
||||||
// exist at runtime but aren't in its type, so feature-detect them to apply
|
// exist at runtime but aren't in its type, so feature-detect them to apply
|
||||||
// backpressure — without it a ~300 MB installer can buffer in memory.
|
// backpressure — without it a ~300 MB installer can buffer in memory.
|
||||||
@@ -221,6 +319,7 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise<A
|
|||||||
response.on('data', (chunk: Buffer) => {
|
response.on('data', (chunk: Buffer) => {
|
||||||
armIdle()
|
armIdle()
|
||||||
received += chunk.length
|
received += chunk.length
|
||||||
|
hash.update(chunk)
|
||||||
if (!stream.write(chunk) && flow.pause && flow.resume) {
|
if (!stream.write(chunk) && flow.pause && flow.resume) {
|
||||||
flow.pause()
|
flow.pause()
|
||||||
stream.once('drain', () => flow.resume?.())
|
stream.once('drain', () => flow.resume?.())
|
||||||
@@ -236,12 +335,23 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise<A
|
|||||||
})
|
})
|
||||||
|
|
||||||
response.on('end', () => {
|
response.on('end', () => {
|
||||||
|
// Body fully received — "stalled" is no longer meaningful past this point.
|
||||||
|
if (idle) clearTimeout(idle)
|
||||||
stream.end(() => {
|
stream.end(() => {
|
||||||
// A truncated download (connection dropped mid-stream) must never be run.
|
// A truncated download (connection dropped mid-stream) must never be run.
|
||||||
if (total !== undefined && received !== total) {
|
if (total !== undefined && received !== total) {
|
||||||
finish({ ok: false, error: 'The download was incomplete — please try again.' })
|
finish({ ok: false, error: 'The download was incomplete — please try again.' })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// Verify the bytes we wrote match the release's published SHA-256.
|
||||||
|
if (expectedSha && hash.digest('hex') !== expectedSha) {
|
||||||
|
finish({
|
||||||
|
ok: false,
|
||||||
|
error:
|
||||||
|
'The downloaded update failed its checksum check — it may be corrupt or tampered with. Nothing was installed.'
|
||||||
|
})
|
||||||
|
return
|
||||||
|
}
|
||||||
finish({ ok: true, filePath })
|
finish({ ok: true, filePath })
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { describe, it, expect } from 'vitest'
|
import { describe, it, expect } from 'vitest'
|
||||||
import { assertHttpUrl } from '../src/main/url'
|
import { assertHttpUrl } from '../src/main/url'
|
||||||
import { isAllowedLoginUrl } from '../src/main/cookies'
|
import { isAllowedLoginUrl } from '../src/main/cookies'
|
||||||
|
import { isTrustedDownloadUrl, extractSha256, compareVersions } from '../src/main/updater'
|
||||||
import { isYtdlpUpdateChannel } from '@shared/ipc'
|
import { isYtdlpUpdateChannel } from '@shared/ipc'
|
||||||
|
|
||||||
// --- F5: assertHttpUrl — argument-injection guard + normalisation -----------
|
// --- F5: assertHttpUrl — argument-injection guard + normalisation -----------
|
||||||
@@ -68,6 +69,88 @@ describe('isAllowedLoginUrl (audit T4)', () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// --- app-updater: isTrustedDownloadUrl — host pin across redirect hops -------
|
||||||
|
|
||||||
|
describe('isTrustedDownloadUrl (app-updater host pin)', () => {
|
||||||
|
const onHost =
|
||||||
|
'https://gitea.netbird.zimspace.uk:5938/debont80/AeroFetch/releases/download/v1/AeroFetch-Setup.exe'
|
||||||
|
|
||||||
|
it('accepts https URLs on the exact update host (incl. port)', () => {
|
||||||
|
expect(isTrustedDownloadUrl(onHost)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects a different host — the redirect/MITM bounce vector', () => {
|
||||||
|
expect(isTrustedDownloadUrl('https://evil.example/AeroFetch-Setup.exe')).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects the right hostname on the wrong port', () => {
|
||||||
|
// host comparison includes the port, so :443 (default) is not the same origin
|
||||||
|
expect(isTrustedDownloadUrl('https://gitea.netbird.zimspace.uk/AeroFetch-Setup.exe')).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects plain http even on the update host', () => {
|
||||||
|
expect(isTrustedDownloadUrl('http://gitea.netbird.zimspace.uk:5938/x.exe')).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('is not fooled by the trusted host placed in the userinfo', () => {
|
||||||
|
expect(
|
||||||
|
isTrustedDownloadUrl('https://gitea.netbird.zimspace.uk:5938@evil.example/x.exe')
|
||||||
|
).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('rejects unparseable input', () => {
|
||||||
|
expect(isTrustedDownloadUrl('not a url')).toBe(false)
|
||||||
|
expect(isTrustedDownloadUrl('')).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
// --- app-updater: extractSha256 — checksum-file parsing ----------------------
|
||||||
|
|
||||||
|
describe('extractSha256 (app-updater checksum parsing)', () => {
|
||||||
|
const hash = 'a'.repeat(64)
|
||||||
|
|
||||||
|
it('reads a bare lowercase digest', () => {
|
||||||
|
expect(extractSha256(hash)).toBe(hash)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('reads sha256sum format (`<hash> filename`)', () => {
|
||||||
|
expect(extractSha256(`${hash} AeroFetch-Setup-0.5.0.exe\n`)).toBe(hash)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('lowercases a PowerShell Get-FileHash (uppercase) digest', () => {
|
||||||
|
expect(extractSha256('A'.repeat(64))).toBe(hash)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('returns null when there is no standalone 64-char hex token', () => {
|
||||||
|
expect(extractSha256('')).toBeNull()
|
||||||
|
expect(extractSha256('not a checksum')).toBeNull()
|
||||||
|
expect(extractSha256('deadbeef')).toBeNull() // too short
|
||||||
|
})
|
||||||
|
|
||||||
|
it('does not slice a 64-char window out of a longer hex run (e.g. sha512)', () => {
|
||||||
|
expect(extractSha256('a'.repeat(128))).toBeNull()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
// --- app-updater: compareVersions — prerelease never outranks its release ----
|
||||||
|
|
||||||
|
describe('compareVersions (app-updater version ordering)', () => {
|
||||||
|
it('orders by numeric components', () => {
|
||||||
|
expect(compareVersions('0.5.0', '0.4.0')).toBe(1)
|
||||||
|
expect(compareVersions('0.4.0', '0.5.0')).toBe(-1)
|
||||||
|
expect(compareVersions('1.2.3', '1.2.3')).toBe(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('treats a leading v as cosmetic', () => {
|
||||||
|
expect(compareVersions('v0.5.0', '0.5.0')).toBe(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('never sorts a prerelease above its final release', () => {
|
||||||
|
// 0.5.0-rc.1 must not be offered as an "upgrade" over a running 0.5.0
|
||||||
|
expect(compareVersions('0.5.0-rc.1', '0.5.0')).toBe(0)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
// --- F1: isYtdlpUpdateChannel — --update-to allowlist -----------------------
|
// --- F1: isYtdlpUpdateChannel — --update-to allowlist -----------------------
|
||||||
|
|
||||||
describe('isYtdlpUpdateChannel (audit F1)', () => {
|
describe('isYtdlpUpdateChannel (audit F1)', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user