diff --git a/src/main/updater.ts b/src/main/updater.ts index 791c49c..1b4e940 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -2,6 +2,7 @@ import { app, net, shell, type WebContents } from 'electron' import { createWriteStream, type WriteStream } from 'fs' import { stat, unlink } from 'fs/promises' import { join, normalize, dirname } from 'path' +import { createHash } from 'crypto' import { IpcChannels, 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 // redirect hop (see downloadAppUpdate), which is what stops a tampered/MITM'd // response from bouncing us off the trusted host. -function isTrustedDownloadUrl(url: string): boolean { +export function isTrustedDownloadUrl(url: string): boolean { try { const u = new URL(url) 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 ab. */ -function compareVersions(a: string, b: string): number { +export function compareVersions(a: string, b: string): number { const pa = parseVersion(a) const pb = parseVersion(b) const len = Math.max(pa.length, pb.length) @@ -82,6 +83,17 @@ interface GiteaRelease { assets?: GiteaAsset[] } +/** + * Pull the SHA-256 hex out of a checksum file — a bare digest, `sha256sum` + * output (` 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. */ function pickInstaller(assets: GiteaAsset[]): GiteaAsset | undefined { const exes = assets.filter((a) => /\.exe$/i.test(a.name)) @@ -146,6 +158,65 @@ export async function checkForAppUpdate(): Promise { /** How long the download may stall (no bytes received) before we give up. */ const DOWNLOAD_IDLE_TIMEOUT_MS = 60_000 +// Integrity: every release MUST attach a checksum asset named `.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. */ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { if (!isTrustedDownloadUrl(url)) { @@ -157,16 +228,44 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { + 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((resolve) => { let settled = false let fileStream: WriteStream | 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 => { if (settled) return settled = true if (idle) clearTimeout(idle) if (!result.ok) { + request.abort() // Close the handle before unlinking (Windows won't delete an open file) // and never leave a partial/aborted installer lying around in temp. if (fileStream && !fileStream.destroyed) fileStream.destroy() @@ -184,14 +283,12 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { if (idle) clearTimeout(idle) idle = setTimeout(() => { - request.abort() finish({ ok: false, error: 'Download stalled — please try again.' }) }, DOWNLOAD_IDLE_TIMEOUT_MS) } request.on('redirect', (_status, _method, redirectUrl) => { if (!isTrustedDownloadUrl(redirectUrl)) { - request.abort() finish({ ok: false, error: 'Refused to follow a redirect to an untrusted location.' }) return } @@ -201,7 +298,6 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { const status = response.statusCode if (status < 200 || status >= 300) { - request.abort() finish({ ok: false, error: `Download failed (HTTP ${status}).` }) return } @@ -211,6 +307,8 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { armIdle() received += chunk.length + hash.update(chunk) if (!stream.write(chunk) && flow.pause && flow.resume) { flow.pause() stream.once('drain', () => flow.resume?.()) @@ -236,12 +335,23 @@ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { + // Body fully received — "stalled" is no longer meaningful past this point. + if (idle) clearTimeout(idle) stream.end(() => { // A truncated download (connection dropped mid-stream) must never be run. if (total !== undefined && received !== total) { finish({ ok: false, error: 'The download was incomplete — please try again.' }) 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 }) }) }) diff --git a/test/security-guards.test.ts b/test/security-guards.test.ts index 3f2bf70..9cae025 100644 --- a/test/security-guards.test.ts +++ b/test/security-guards.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest' import { assertHttpUrl } from '../src/main/url' import { isAllowedLoginUrl } from '../src/main/cookies' +import { isTrustedDownloadUrl, extractSha256, compareVersions } from '../src/main/updater' import { isYtdlpUpdateChannel } from '@shared/ipc' // --- 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 (` 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 ----------------------- describe('isYtdlpUpdateChannel (audit F1)', () => {