security: harden command exec, IPC, deep-link, cookies, and persistence
Seven-tier security audit of the main process, each finding fixed with a
regression test. Typecheck (node + web) clean; unit tests 106 -> 140.
- Tier 1 (command exec/argv): allowlist the yt-dlp --update-to channel
(blocks arbitrary-binary-install RCE); gate per-download extraArgs behind
the customCommandEnabled consent flag in main (blocks --exec RCE); resolve
taskkill/schtasks by absolute System32 path; validate per-download
outputDir; normalize the URL in assertHttpUrl and use it at every spawn.
- Tier 2 (input validation): fix isSafeFilenameTemplate drive-relative
('C:foo') and Windows dotted-'..' traversal bypasses; percent-encode the
untrusted id in entryUrl; catch reserved device names with extensions in
sanitizeDirSegment.
- Tier 3 (fs/backup): drop malformed template rows in importBackup;
normalize deep-link URLs via assertHttpUrl.
- Tier 4 (cookies): confine the sign-in window's navigations/popups to web
URLs (recursively) and deny all web permissions on its session.
- Tier 5 (deep-link/argv): bound the .url file read to 64 KB; match the
aerofetch:// scheme case-insensitively.
- Tier 6 (Electron window): deny camera/mic/geolocation/USB/HID/serial/
Bluetooth permissions on the app window.
- Tier 7 (network/persistence): restrict the watched-source RSS fetch to
youtube.com feed URLs (SSRF guard); complete isValidSource validation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+50
-17
@@ -2,14 +2,22 @@ import { spawn, execFile, type ChildProcess } from 'child_process'
|
||||
import { existsSync } from 'fs'
|
||||
import { join } from 'path'
|
||||
import { app, BrowserWindow, Notification, type WebContents } from 'electron'
|
||||
import { getYtdlpPath, getBinDir, getAria2cPath, getFfmpegPath, getFfprobePath } from './binaries'
|
||||
import {
|
||||
getYtdlpPath,
|
||||
getBinDir,
|
||||
getAria2cPath,
|
||||
getFfmpegPath,
|
||||
getFfprobePath,
|
||||
getSystem32Path
|
||||
} from './binaries'
|
||||
import { getSettings, getDownloadArchivePath } from './settings'
|
||||
import { getCookiesFilePath } from './cookies'
|
||||
import { listTemplates } from './templates'
|
||||
import { assertHttpUrl } from './url'
|
||||
import { isSafeOutputDir } from './validation'
|
||||
import {
|
||||
buildArgs,
|
||||
parseExtraArgs,
|
||||
selectExtraArgs,
|
||||
formatCommandLine,
|
||||
collectionOutputTemplate
|
||||
} from './buildArgs'
|
||||
@@ -151,21 +159,32 @@ function probeMeta(ytdlp: string, url: string): Promise<DownloadMeta | null> {
|
||||
|
||||
// --- Argv construction (shared by startDownload and the command preview) ---
|
||||
|
||||
// A per-download override (opts.extraArgs, even '') always wins; otherwise
|
||||
// fall back to the persisted default template when custom-command mode is on.
|
||||
// A per-download override (opts.extraArgs, even '') wins over the persisted
|
||||
// default template — but BOTH are gated on the customCommandEnabled consent flag
|
||||
// (see selectExtraArgs / audit F2). The gate is enforced here in main, not just
|
||||
// in the renderer UI, so the renderer can't be trusted to apply it.
|
||||
function resolveExtraArgs(opts: StartDownloadOptions, settings: Settings): string[] {
|
||||
if (opts.extraArgs !== undefined) return parseExtraArgs(opts.extraArgs)
|
||||
if (settings.customCommandEnabled && settings.defaultTemplateId) {
|
||||
const tpl = listTemplates().find((t) => t.id === settings.defaultTemplateId)
|
||||
if (tpl) return parseExtraArgs(tpl.args)
|
||||
}
|
||||
return []
|
||||
return selectExtraArgs({
|
||||
customCommandEnabled: settings.customCommandEnabled,
|
||||
perDownloadExtraArgs: opts.extraArgs,
|
||||
defaultTemplateId: settings.defaultTemplateId,
|
||||
templates: listTemplates()
|
||||
})
|
||||
}
|
||||
|
||||
/** Resolve settings + per-download overrides into the full yt-dlp argv. */
|
||||
export function buildCommand(opts: StartDownloadOptions): string[] {
|
||||
const settings = getSettings()
|
||||
const outDir = opts.outputDir?.trim() || settings.outputDir || app.getPath('downloads')
|
||||
// A per-download outputDir override must clear the same safety check the
|
||||
// persisted setting does (absolute path only); a renderer-supplied override is
|
||||
// otherwise untrusted — it dictates where downloaded files get written. An
|
||||
// unsafe override is ignored in favour of the validated setting, then the OS
|
||||
// Downloads folder. (audit F4)
|
||||
const override = opts.outputDir?.trim()
|
||||
const outDir =
|
||||
(override && isSafeOutputDir(override) ? override : '') ||
|
||||
settings.outputDir ||
|
||||
app.getPath('downloads')
|
||||
// A collection (media-manager) download is filed into <channel>/<playlist>/
|
||||
// <NNN> - <title> folders; an ordinary download uses the flat filenameTemplate.
|
||||
const filenameTemplate = settings.filenameTemplate?.trim() || '%(title)s.%(ext)s'
|
||||
@@ -198,13 +217,16 @@ export function buildCommand(opts: StartDownloadOptions): string[] {
|
||||
|
||||
/** Build the exact command line for the current form state, without running it. */
|
||||
export function previewCommand(opts: StartDownloadOptions): CommandPreviewResult {
|
||||
let normalized: StartDownloadOptions
|
||||
try {
|
||||
assertHttpUrl(opts.url)
|
||||
// Use the parser-normalised URL (audit F5), so the previewed command matches
|
||||
// exactly what startDownload would spawn.
|
||||
normalized = { ...opts, url: assertHttpUrl(opts.url) }
|
||||
} catch (e) {
|
||||
return { ok: false, error: (e as Error).message }
|
||||
}
|
||||
try {
|
||||
return { ok: true, command: formatCommandLine(getYtdlpPath(), buildCommand(opts)) }
|
||||
return { ok: true, command: formatCommandLine(getYtdlpPath(), buildCommand(normalized)) }
|
||||
} catch (e) {
|
||||
return { ok: false, error: (e as Error).message }
|
||||
}
|
||||
@@ -239,9 +261,13 @@ export function startDownload(
|
||||
`Add the ffmpeg build's binaries to resources/bin/ (see the README there).`
|
||||
}
|
||||
}
|
||||
// Reject anything that isn't an http(s) URL before it reaches yt-dlp's argv.
|
||||
// Reject anything that isn't an http(s) URL before it reaches yt-dlp's argv,
|
||||
// and replace opts.url with the parser-normalised form so the exact string we
|
||||
// validated is the one that gets spawned/probed — not a raw variant carrying
|
||||
// interior tabs/newlines or leading control chars that URL parsing silently
|
||||
// tolerates. (audit F5)
|
||||
try {
|
||||
assertHttpUrl(opts.url)
|
||||
opts = { ...opts, url: assertHttpUrl(opts.url) }
|
||||
} catch (e) {
|
||||
return { ok: false, error: (e as Error).message }
|
||||
}
|
||||
@@ -344,8 +370,15 @@ export function cancelDownload(id: string): void {
|
||||
rec.canceled = true
|
||||
const pid = rec.child.pid
|
||||
if (pid != null) {
|
||||
// Kill the whole tree (/T) so the spawned ffmpeg child dies too.
|
||||
execFile('taskkill', ['/pid', String(pid), '/T', '/F'], { windowsHide: true }, () => {})
|
||||
// Kill the whole tree (/T) so the spawned ffmpeg child dies too. Resolve
|
||||
// taskkill from System32 by absolute path, never the bare name, so a planted
|
||||
// taskkill.exe on PATH / in the CWD can't run in its place. (audit F3)
|
||||
execFile(
|
||||
getSystem32Path('taskkill.exe'),
|
||||
['/pid', String(pid), '/T', '/F'],
|
||||
{ windowsHide: true },
|
||||
() => {}
|
||||
)
|
||||
} else {
|
||||
rec.child.kill()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user