diff --git a/cloakbrowser/config.py b/cloakbrowser/config.py index fb1ed6ed..c6c46213 100644 --- a/cloakbrowser/config.py +++ b/cloakbrowser/config.py @@ -5,10 +5,21 @@ import os import platform import random +import re from pathlib import Path from ._version import __version__ +# A Chromium version tag is a plain dotted-numeric string (e.g. "146.0.7680.177.5"). +# Validate any externally sourced version (GitHub release tags, cache markers) +# before it flows into cache paths or download URLs. +_SAFE_VERSION_RE = re.compile(r"^[0-9]+(\.[0-9]+){0,9}$") + + +def is_safe_version_tag(version: str) -> bool: + """Return True if *version* is a safe dotted-numeric version string.""" + return isinstance(version, str) and bool(_SAFE_VERSION_RE.match(version)) + # --------------------------------------------------------------------------- # Chromium version shipped with this release. # Different platforms may ship different versions during transition periods. @@ -170,7 +181,7 @@ def get_effective_version() -> str: if marker.exists(): try: version = marker.read_text().strip() - if version and _version_newer(version, base): + if version and is_safe_version_tag(version) and _version_newer(version, base): binary = get_binary_path(version) if binary.exists(): return version diff --git a/cloakbrowser/download.py b/cloakbrowser/download.py index fc064eb8..0d1955a7 100644 --- a/cloakbrowser/download.py +++ b/cloakbrowser/download.py @@ -29,6 +29,7 @@ GITHUB_DOWNLOAD_BASE_URL, _version_newer, check_platform_available, + is_safe_version_tag, get_archive_ext, get_archive_name, get_binary_dir, @@ -174,18 +175,35 @@ def _download_and_extract(version: str | None = None) -> None: def _verify_download_checksum(file_path: Path, version: str | None = None) -> None: - """Fetch SHA256SUMS and verify the downloaded file. Warn if unavailable, fail on mismatch.""" + """Fetch SHA256SUMS and verify the downloaded file. + + Fails closed: if checksums cannot be fetched, or contain no entry for this + platform's archive, the binary is treated as unverified and the download is + rejected. Set ``CLOAKBROWSER_SKIP_CHECKSUM=true`` to bypass at your own risk + (e.g. a custom mirror that does not publish SHA256SUMS). The official + releases publish SHA256SUMS for every version, so this only triggers when a + mirror is misconfigured or the file was suppressed/tampered with in transit. + """ checksums = _fetch_checksums(version) tarball_name = get_archive_name() if checksums is None: - logger.warning("SHA256SUMS not available for this release — skipping checksum verification") - return + raise RuntimeError( + f"Could not fetch SHA256SUMS for this release, so the downloaded binary " + f"({tarball_name}) cannot be integrity-verified. Refusing to use an " + f"unverified binary. If you are using a custom CLOAKBROWSER_DOWNLOAD_URL, " + f"publish a SHA256SUMS file alongside the archive, or set " + f"CLOAKBROWSER_SKIP_CHECKSUM=true to bypass this check at your own risk." + ) expected = checksums.get(tarball_name) if expected is None: - logger.warning("SHA256SUMS found but no entry for %s — skipping verification", tarball_name) - return + raise RuntimeError( + f"SHA256SUMS was fetched but contains no entry for {tarball_name}, so the " + f"downloaded binary cannot be integrity-verified. Refusing to use an " + f"unverified binary. Set CLOAKBROWSER_SKIP_CHECKSUM=true to bypass at your " + f"own risk." + ) _verify_checksum(file_path, expected) @@ -204,14 +222,36 @@ def _fetch_checksums(version: str | None = None) -> dict[str, str] | None: try: resp = httpx.get(url, follow_redirects=True, timeout=10.0) resp.raise_for_status() - return _parse_checksums(resp.text) + parsed = _parse_checksums(resp.text) + # A redirect to an HTML error/login page (or any non-SHA256SUMS body) + # parses to an empty mapping. Treat that as "not found here" and try + # the next mirror, so a misbehaving primary can't shadow a good + # fallback (and can't satisfy the fail-closed check with junk). + if parsed: + return parsed except Exception: continue return None +def _is_sha256_hex(value: str) -> bool: + """True if *value* is exactly a 64-character hex SHA-256 digest.""" + if len(value) != 64: + return False + try: + int(value, 16) + return True + except ValueError: + return False + + def _parse_checksums(text: str) -> dict[str, str]: - """Parse SHA256SUMS format: 'hash filename' per line.""" + """Parse SHA256SUMS format: 'hash filename' per line. + + Only accepts lines whose first token is a valid 64-char hex SHA-256 digest, + so an HTML error page or other non-checksum body parses to an empty mapping + instead of producing bogus entries. + """ result = {} for line in text.strip().splitlines(): line = line.strip() @@ -220,6 +260,8 @@ def _parse_checksums(text: str) -> dict[str, str]: parts = line.split(None, 1) if len(parts) == 2: hash_val, filename = parts + if not _is_sha256_hex(hash_val): + continue filename = filename.lstrip("*") result[filename] = hash_val.lower() return result @@ -478,9 +520,17 @@ def _get_latest_chromium_version() -> str | None: for release in resp.json(): tag = release.get("tag_name", "") if tag.startswith("chromium-v") and not release.get("draft"): + version = tag.removeprefix("chromium-v") + # Defense in depth: the version flows into cache paths and + # download URLs. Reject anything that isn't a plain dotted-numeric + # version so a malformed/hostile release tag can't inject path or + # URL components. + if not is_safe_version_tag(version): + logger.debug("Skipping release with unsafe version tag: %r", tag) + continue asset_names = {a["name"] for a in release.get("assets", [])} if platform_tarball in asset_names: - return tag.removeprefix("chromium-v") + return version return None except Exception: logger.debug("Auto-update check failed", exc_info=True) diff --git a/js/src/config.ts b/js/src/config.ts index 4b73057f..484568a3 100644 --- a/js/src/config.ts +++ b/js/src/config.ts @@ -154,7 +154,7 @@ export function getEffectiveVersion(): string { try { if (fs.existsSync(marker)) { const version = fs.readFileSync(marker, "utf-8").trim(); - if (version && versionNewer(version, base)) { + if (version && isSafeVersionTag(version) && versionNewer(version, base)) { const binary = getBinaryPath(version); if (fs.existsSync(binary)) { return version; @@ -168,6 +168,15 @@ export function getEffectiveVersion(): string { return base; } +// A Chromium version tag is a plain dotted-numeric string (e.g. "146.0.7680.177.5"). +// Validate any externally sourced version (GitHub release tags, cache markers) +// before it flows into cache paths, download URLs, or shell/extraction commands. +const SAFE_VERSION_RE = /^[0-9]+(\.[0-9]+){0,9}$/; + +export function isSafeVersionTag(version: string): boolean { + return typeof version === "string" && SAFE_VERSION_RE.test(version); +} + export function parseVersion(v: string): number[] { return v.split(".").map(Number); } diff --git a/js/src/download.ts b/js/src/download.ts index c4f5f3ac..d27100ed 100644 --- a/js/src/download.ts +++ b/js/src/download.ts @@ -30,6 +30,7 @@ import { getFallbackDownloadUrl, getLocalBinaryOverride, getPlatformTag, + isSafeVersionTag, versionNewer, } from "./config.js"; @@ -211,18 +212,29 @@ async function downloadAndExtract(version?: string): Promise { } async function verifyDownloadChecksum(filePath: string, version?: string): Promise { + // Fails closed: a missing SHA256SUMS, or one with no entry for this platform's + // archive, means the binary is unverified and the download is rejected. Set + // CLOAKBROWSER_SKIP_CHECKSUM=true to bypass (handled by the caller). const checksums = await fetchChecksums(version); const tarballName = getArchiveName(); if (!checksums) { - console.warn("[cloakbrowser] SHA256SUMS not available for this release — skipping checksum verification"); - return; + throw new Error( + `Could not fetch SHA256SUMS for this release, so the downloaded binary ` + + `(${tarballName}) cannot be integrity-verified. Refusing to use an unverified ` + + `binary. If you are using a custom CLOAKBROWSER_DOWNLOAD_URL, publish a ` + + `SHA256SUMS file alongside the archive, or set CLOAKBROWSER_SKIP_CHECKSUM=true ` + + `to bypass this check at your own risk.` + ); } const expected = checksums.get(tarballName); if (!expected) { - console.warn(`[cloakbrowser] SHA256SUMS found but no entry for ${tarballName} — skipping verification`); - return; + throw new Error( + `SHA256SUMS was fetched but contains no entry for ${tarballName}, so the ` + + `downloaded binary cannot be integrity-verified. Refusing to use an unverified ` + + `binary. Set CLOAKBROWSER_SKIP_CHECKSUM=true to bypass at your own risk.` + ); } await verifyChecksum(filePath, expected); @@ -246,7 +258,12 @@ export async function fetchChecksums(version?: string): Promise 0) return parsed; } catch { continue; } @@ -424,12 +441,19 @@ async function extractZip(archivePath: string, destDir: string): Promise { if (process.platform === "win32") { // PowerShell 5.1's Expand-Archive uses .NET FileStream which can conflict - // with recently-closed Node.js file handles. Use ZipFile API directly. + // with recently-closed Node.js file handles. Use the ZipFile API directly. + // Pass the paths via environment variables rather than interpolating them + // into the -Command string, so a path containing a single quote (or other + // PowerShell metacharacter) cannot break out of the literal and execute as + // code. The paths are read inside PowerShell as $env: values (plain data). execFileSync("powershell", [ "-NoProfile", "-Command", `Add-Type -AssemblyName System.IO.Compression.FileSystem; ` + - `[System.IO.Compression.ZipFile]::ExtractToDirectory('${archivePath}', '${destDir}')`, - ], { timeout: 120_000 }); + `[System.IO.Compression.ZipFile]::ExtractToDirectory($env:CLOAKBROWSER_ZIP_SRC, $env:CLOAKBROWSER_ZIP_DEST)`, + ], { + timeout: 120_000, + env: { ...process.env, CLOAKBROWSER_ZIP_SRC: archivePath, CLOAKBROWSER_ZIP_DEST: destDir }, + }); } else { execFileSync("unzip", ["-o", archivePath, "-d", destDir], { timeout: 120_000 }); } @@ -511,11 +535,17 @@ export async function getLatestChromiumVersion(): Promise { const platformTarball = getArchiveName(); for (const release of releases) { if (release.tag_name.startsWith("chromium-v") && !release.draft) { + const version = release.tag_name.replace(/^chromium-v/, ""); + // Defense in depth: the version flows into cache paths, download URLs, + // and the Windows zip-extraction command. Reject anything that isn't a + // plain dotted-numeric version so a malformed/hostile release tag can't + // inject path, URL, or command components. + if (!isSafeVersionTag(version)) continue; const assetNames = new Set( (release.assets ?? []).map((a) => a.name) ); if (assetNames.has(platformTarball)) { - return release.tag_name.replace(/^chromium-v/, ""); + return version; } } } diff --git a/tests/test_supplychain_hardening.py b/tests/test_supplychain_hardening.py new file mode 100644 index 00000000..d6f04065 --- /dev/null +++ b/tests/test_supplychain_hardening.py @@ -0,0 +1,85 @@ +"""Tests for binary supply-chain hardening. + + * checksum verification fails CLOSED on missing/absent SHA256SUMS + * _fetch_checksums skips non-checksum (HTML) bodies and falls through mirrors + * _parse_checksums only accepts valid 64-hex digests + * version-tag validation (is_safe_version_tag) blocks injection-shaped tags +""" + +from unittest.mock import MagicMock, patch + +import pytest + +import cloakbrowser.download as download +from cloakbrowser.config import get_archive_name, is_safe_version_tag + + +class TestChecksumFailClosed: + def test_raises_when_checksums_unavailable(self, tmp_path): + f = tmp_path / "archive" + f.write_bytes(b"unverified") + with patch("cloakbrowser.download._fetch_checksums", return_value=None): + with pytest.raises(RuntimeError, match="cannot be integrity-verified"): + download._verify_download_checksum(f) + + def test_raises_when_no_entry_for_platform(self, tmp_path): + f = tmp_path / "archive" + f.write_bytes(b"unverified") + with patch("cloakbrowser.download._fetch_checksums", + return_value={"some-other-file.tar.gz": "00" * 32}): + with pytest.raises(RuntimeError, match="no entry for"): + download._verify_download_checksum(f) + + def test_passes_when_entry_matches(self, tmp_path): + import hashlib + content = b"the real binary bytes" + f = tmp_path / "archive" + f.write_bytes(content) + digest = hashlib.sha256(content).hexdigest() + with patch("cloakbrowser.download._fetch_checksums", + return_value={get_archive_name(): digest}): + download._verify_download_checksum(f) # should not raise + + +class TestFetchChecksumsSkipsJunk: + def test_html_body_is_skipped_in_favor_of_valid_mirror(self): + valid = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 file.tar.gz\n" + + def mock_get(url, **kwargs): + resp = MagicMock() + resp.raise_for_status = MagicMock() + resp.text = ("404 Not Found" + if "cloakbrowser.dev" in url else valid) + return resp + + with patch.dict("os.environ", {"CLOAKBROWSER_DOWNLOAD_URL": ""}): + with patch("cloakbrowser.download.httpx.get", side_effect=mock_get): + result = download._fetch_checksums() + assert result == {"file.tar.gz": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"} + + def test_parse_rejects_non_hex_lines(self): + text = "notahash evil.tar.gz\nGGGG also-bad.tar.gz\n" + assert download._parse_checksums(text) == {} + + +class TestVersionTagValidation: + @pytest.mark.parametrize("v", ["146.0.7680.177.5", "145.0.0.0", "1", "1.2.3.4.5"]) + def test_valid_versions(self, v): + assert is_safe_version_tag(v) + + @pytest.mark.parametrize("v", [ + "", "../../etc", "146.0; calc", "146'+x", "v1.2.3", "1.2.3-beta", "1.2/3", + ]) + def test_invalid_versions(self, v): + assert not is_safe_version_tag(v) + + def test_get_latest_skips_unsafe_tag(self): + releases = [ + {"tag_name": "chromium-v9'; evil", "draft": False, + "assets": [{"name": get_archive_name()}]}, + ] + resp = MagicMock() + resp.json.return_value = releases + resp.raise_for_status = MagicMock() + with patch("cloakbrowser.download.httpx.get", return_value=resp): + assert download._get_latest_chromium_version() is None