9bd9b6bedb2c
H-08: strict SemVer, path-traversal guard, byte cap, APK identity check, no sole-artifact fallback
- time
- 2026-07-29 13:54 UTC · 2 months ago
- machine
- koh2
- agent
- claude-opus-4
- session
- audit-remediation-2026-07-29
- parent
- a0af1c52d84b
↓ peach-android-9bd9b6be.tar.gzatlas at this save
files changed3 files
app/src/main/java/com/peachpasswords/updates/UpdateManifest.kt+62 −4app/src/main/java/com/peachpasswords/updates/UpdateRepository.kt+75 −11app/src/test/java/com/peachpasswords/updates/UpdateManifestTest.kt+113 −0app/src/main/java/com/peachpasswords/updates/UpdateManifest.kt+62 −4
@@ -46,6 +46,12 @@4646 4747 class ManifestFormatException(message: String) : Exception(message)4848 49+/** Thrown when the APK on disk is not the package/version the updater was told to install. */50+class ApkIdentityException(message: String) : Exception(message)51+52+/** Thrown when the download would exceed [MAX_APK_BYTES] or advertise a larger Content-Length. */53+class DownloadSizeException(message: String) : Exception(message)54+4955 /**5056 * SETTLED — Kepr accepts `android-arm64` as of kepr v1.56.237, matching its5157 * `<os>-<arch>` platform vocabulary. This is the key the release manifest is@@ -56,6 +62,54 @@5662 const val ANDROID_PLATFORM_KEY = "android-arm64"5763 5864 /**65+ * Hard ceiling on a downloaded APK. Kepr publishes ~24 MB today; 50 MB leaves66+ * headroom for growth without letting a hostile manifest exhaust cache or disk.67+ */68+const val MAX_APK_BYTES = 50L * 1024 * 102469+70+/**71+ * Strict SemVer core: exactly three non-empty, non-leading-zero decimal72+ * components, each small enough to fit in an Int. Anything else (NaN segments,73+ * extra components, negatives, overflow) is refused — the manifest's version74+ * is later used to derive the cache filename, so a permissive parser here would75+ * let `..` or `/` reach the filesystem.76+ */77+private val STRICT_SEMVER = Regex("""^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)$""")78+79+internal fun validateSemver(version: String) {80+ if (!STRICT_SEMVER.matches(version)) {81+ throw ManifestFormatException("version '$version' is not strict SemVer major.minor.patch")82+ }83+ version.split('.').forEach { seg ->84+ // STRICT_SEMVER already excludes non-digits and leading zeros, so this85+ // overflow check is the only thing left standing between "9999999999"86+ // and a silently-wrapped compareVersions result.87+ if (seg.length > 9 || seg.toLong() > Int.MAX_VALUE) {88+ throw ManifestFormatException("version '$version' has an out-of-range segment")89+ }90+ }91+}92+93+/**94+ * True when a parsed APK archive is the package and version this updater95+ * belongs to. Pure: takes the values PackageManager already extracted so the96+ * Android side stays at the call site and this stays unit-testable.97+ *98+ * Both the production package (`com.peachpasswords`) and the debug suffix99+ * variant (`com.peachpasswords.debug`) are accepted so debug builds can still100+ * install their own updates; anything else is rejected outright.101+ */102+fun isExpectedApk(103+ packageName: String?,104+ versionCode: Long,105+ expectedPackage: String,106+ expectedVersionCode: Int,107+): Boolean {108+ if (packageName != expectedPackage && packageName != "$expectedPackage.debug") return false109+ return versionCode >= expectedVersionCode110+}111+112+/**59113 * Parse a Kepr release manifest.60114 *61115 * Two contract wrinkles are handled deliberately:@@ -64,6 +118,13 @@64118 * - `optString` on a JSON null yields the literal string "null" in org.json, which65119 * would render as changelog text. [optTextOrNull] rejects that class of value66120 * outright rather than trusting the caller to notice.121+ *122+ * Security note (H-08): the canonical platform key is required and the123+ * sole-artifact fallback is gone. A Kepr build that only published, say,124+ * `android-x86` would have been accepted before and pointed an Android phone at125+ * the wrong binary; now it is rejected outright. A signed release-manifest126+ * channel is the real fix — that is an architectural change and is out of scope127+ * for H-08.67128 */68129 fun parseReleaseManifest(body: String, platformKey: String = ANDROID_PLATFORM_KEY): ReleaseManifest {69130 val root = runCatching { JSONObject(body) }@@ -71,15 +132,12 @@71132 72133 val version = root.optTextOrNull("version")73134 ?: throw ManifestFormatException("manifest has no version")135+ validateSemver(version)74136 75137 val platforms = root.optJSONObject("platforms")76138 ?: throw ManifestFormatException("manifest has no platforms")77139 78- // Fall back to the sole platform when the key doesn't match: a single-artifact79- // Android release is unambiguous, and a naming mismatch shouldn't strand every80- // installed build with "no download available".81140 val platform = platforms.optJSONObject(platformKey)82- ?: platforms.keys().asSequence().toList().singleOrNull()?.let { platforms.optJSONObject(it) }83141 ?: throw ManifestFormatException("manifest has no '$platformKey' artifact")84142 85143 val url = platform.optTextOrNull("url")app/src/main/java/com/peachpasswords/updates/UpdateRepository.kt+75 −11
@@ -2,6 +2,7 @@22 33 import android.content.Context44 import android.content.Intent5+import android.content.pm.PackageManager56 import android.net.Uri67 import android.provider.Settings78 import androidx.core.content.FileProvider@@ -125,13 +126,23 @@125126 val expected = parseChecksum(manifest.checksum)126127 ?: throw ChecksumMismatchException("unrecognised checksum '${manifest.checksum}'")127128 129+ // parseReleaseManifest has already enforced strict SemVer, so the version130+ // is guaranteed to be a clean `major.minor.patch` with no path-traversal131+ // characters; using it in the filename is safe.128132 val dir = File(context.cacheDir, CACHE_DIR).apply { mkdirs() }129133 val target = File(dir, "peach-${manifest.version}.apk")134+ require(!target.name.contains("..") && !target.name.contains('/') && !target.name.contains('\\')) {135+ "refusing to write outside cache dir"136+ }130137 131138 // A previously verified download of the same version is reusable; anything132139 // else in here is a stale or half-finished attempt.133140 dir.listFiles()?.forEach { if (it != target) it.delete() }134141 if (target.exists() && hashFile(target, expected.algorithm) == expected.hex) {142+ // Even on a cache hit we re-prove identity: a tampered file in the143+ // cache slot must not reach the installer any more than a tampered144+ // download must.145+ verifyApkIdentity(target)135146 onProgress(1f)136147 return@withContext target137148 }@@ -144,19 +155,43 @@144155 client.newCall(request).execute().use { response ->145156 if (!response.isSuccessful) throw IOException("HTTP ${response.code}")146157 val body = response.body ?: throw IOException("empty response")147- val total = body.contentLength()148- body.byteStream().use { input ->149- target.outputStream().use { output ->150- val buffer = ByteArray(64 * 1024)151- var written = 0L152- while (true) {153- val read = input.read(buffer)154- if (read <= 0) break155- output.write(buffer, 0, read)156- written += read157- if (total > 0) onProgress((written.toFloat() / total).coerceIn(0f, 1f))158+ val advertised = body.contentLength()159+ // Reject before streaming if the server is honest about being too160+ // big; Content-Length can be -1 (chunked / unknown) and the161+ // streaming cap below catches that case as bytes arrive.162+ if (advertised in 1..MAX_APK_BYTES) {163+ // known and within bounds — nothing to do164+ } else if (advertised > MAX_APK_BYTES) {165+ throw DownloadSizeException("APK advertises $advertised bytes, cap is $MAX_APK_BYTES")166+ }167+ try {168+ body.byteStream().use { input ->169+ target.outputStream().use { output ->170+ val buffer = ByteArray(64 * 1024)171+ var written = 0L172+ while (true) {173+ val read = input.read(buffer)174+ if (read <= 0) break175+ written += read176+ if (written > MAX_APK_BYTES) {177+ // Streaming cap: a server that lies about its178+ // Content-Length, or chunks past it, is still179+ // bounded here. The partial file is removed so180+ // a later retry doesn't pick up half an APK.181+ throw DownloadSizeException(182+ "APK exceeded $MAX_APK_BYTES bytes mid-download",183+ )184+ }185+ output.write(buffer, 0, read)186+ if (advertised > 0) onProgress(187+ (written.toFloat() / advertised).coerceIn(0f, 1f),188+ )189+ }158190 }159191 }192+ } catch (e: DownloadSizeException) {193+ target.delete()194+ throw e160195 }161196 }162197 @@ -167,8 +202,37 @@167202 "${expected.algorithm} mismatch — expected ${expected.hex}, got $actual",168203 )169204 }205+ verifyApkIdentity(target)170206 onProgress(1f)171207 target208+ }209+210+ /**211+ * Confirm the cached APK is the package and version this updater belongs212+ * to. PackageManager is happy to parse any well-formed APK the cache may213+ * contain (or, in the future, a hostile Kepr build with a substituted214+ * manifest); the package-name and version-code checks are what stop an215+ * arbitrary binary from being handed to the installer.216+ */217+ private fun verifyApkIdentity(apk: File) {218+ val info = context.packageManager.getPackageArchiveInfo(219+ apk.absolutePath,220+ 0,221+ ) ?: throw ApkIdentityException("APK could not be parsed as an Android package")222+ if (!isExpectedApk(223+ packageName = info.packageName,224+ versionCode = info.longVersionCode,225+ expectedPackage = context.packageName,226+ expectedVersionCode = BuildConfig.VERSION_CODE,227+ )228+ ) {229+ apk.delete()230+ throw ApkIdentityException(231+ "APK identity mismatch: package=${info.packageName} " +232+ "versionCode=${info.longVersionCode}, expected ${context.packageName} " +233+ ">=${BuildConfig.VERSION_CODE}",234+ )235+ }172236 }173237 174238 fun discardDownloads() {app/src/test/java/com/peachpasswords/updates/UpdateManifestTest.kt+113 −0
@@ -46,6 +46,9 @@4646 4747 @Test4848 fun `missing platform falls back to the sole artifact`() {49+ // H-08: the sole-artifact fallback is gone. The canonical50+ // android-arm64 key is the only one the updater will accept — but the51+ // test fixture is that key, so the canonical path still passes.4952 val body = """5053 {"version":"2.3.68","platforms":{"android-arm64":5154 {"url":"https://x/y","checksum":"sha256:${"b".repeat(64)}"}}}@@ -58,7 +61,90 @@5861 parseReleaseManifest("""{"version":"2.3.68"}""")5962 }6063 64+ @Test(expected = ManifestFormatException::class)65+ fun `non-canonical platform key is rejected, no sole-artifact fallback`() {66+ // A Kepr build that only published android-x86 used to be accepted via67+ // the sole-artifact fallback; now it is refused outright.68+ val body = """69+ {"version":"2.3.68","platforms":{"android-x86":70+ {"url":"https://x/y","checksum":"sha256:${"b".repeat(64)}"}}}71+ """.trimIndent()72+ parseReleaseManifest(body)73+ }74+75+ /* ────────────────────────── strict SemVer ────────────────────────── */76+6177 @Test78+ fun `parseReleaseManifest accepts a clean SemVer version`() {79+ val parsed = parseReleaseManifest(manifest(""))80+ assertEquals("2.3.68", parsed.version)81+ }82+83+ @Test(expected = ManifestFormatException::class)84+ fun `version with only two components is rejected`() {85+ parseReleaseManifest(86+ """{"version":"2.3","platforms":{"$ANDROID_PLATFORM_KEY":87+ {"url":"u","checksum":"blake3:${"a".repeat(64)}"}}}""",88+ )89+ }90+91+ @Test(expected = ManifestFormatException::class)92+ fun `version with four components is rejected`() {93+ parseReleaseManifest(94+ """{"version":"2.3.68.0","platforms":{"$ANDROID_PLATFORM_KEY":95+ {"url":"u","checksum":"blake3:${"a".repeat(64)}"}}}""",96+ )97+ }98+99+ @Test(expected = ManifestFormatException::class)100+ fun `version with path-traversal characters is rejected`() {101+ parseReleaseManifest(102+ """{"version":"../../../etc/passwd","platforms":{"$ANDROID_PLATFORM_KEY":103+ {"url":"u","checksum":"blake3:${"a".repeat(64)}"}}}""",104+ )105+ }106+107+ @Test(expected = ManifestFormatException::class)108+ fun `version with a leading zero segment is rejected`() {109+ // Strict SemVer: 02 is not a valid integer literal, so 02.3.68 is not110+ // a valid version string — the filesystem would also sort it oddly.111+ parseReleaseManifest(112+ """{"version":"02.3.68","platforms":{"$ANDROID_PLATFORM_KEY":113+ {"url":"u","checksum":"blake3:${"a".repeat(64)}"}}}""",114+ )115+ }116+117+ @Test(expected = ManifestFormatException::class)118+ fun `version with an overflowing segment is rejected`() {119+ // 99_999_999_999 would silently wrap in compareVersions' toIntOrNull;120+ // strict validation refuses it outright.121+ parseReleaseManifest(122+ """{"version":"99999999999.0.0","platforms":{"$ANDROID_PLATFORM_KEY":123+ {"url":"u","checksum":"blake3:${"a".repeat(64)}"}}}""",124+ )125+ }126+127+ @Test(expected = ManifestFormatException::class)128+ fun `version with non-numeric components is rejected`() {129+ parseReleaseManifest(130+ """{"version":"2.3.NaN","platforms":{"$ANDROID_PLATFORM_KEY":131+ {"url":"u","checksum":"blake3:${"a".repeat(64)}"}}}""",132+ )133+ }134+135+ /* ─────────────────────────────── size cap ─────────────────────────────── */136+137+ @Test138+ fun `MAX_APK_BYTES is a hard ceiling an advertised Content-Length can be compared against`() {139+ // The streaming branch checks Content-Length before reading bytes; the140+ // simplest invariant is that the constant is positive and the well-141+ // known 50 MiB. Locking the value here so any silent change has to142+ // update this assertion.143+ assertEquals(50L * 1024 * 1024, MAX_APK_BYTES)144+ assertTrue(MAX_APK_BYTES > 0)145+ }146+147+ @Test62148 fun `version comparison is numeric, not lexical`() {63149 assertTrue(isNewerVersion("2.3.100", "2.3.67"))64150 assertTrue(isNewerVersion("2.4.0", "2.3.99"))@@ -107,5 +193,32 @@107193 "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad",108194 hashStream("abc".toByteArray().inputStream(), "sha256"),109195 )196+ }197+198+ /** APK identity gate — must accept only this app's package at this app's version-or-newer. */199+ @Test200+ fun `isExpectedApk accepts production and debug packages at or above the running version`() {201+ assertTrue(isExpectedApk("com.peachpasswords", 240146L, "com.peachpasswords", 240145))202+ assertTrue(isExpectedApk("com.peachpasswords", 240145L, "com.peachpasswords", 240145))203+ assertTrue(isExpectedApk("com.peachpasswords.debug", 240200L, "com.peachpasswords", 240145))204+ }205+206+ @Test207+ fun `isExpectedApk rejects a foreign package`() {208+ assertFalse(isExpectedApk("com.evil.twin", 999_999L, "com.peachpasswords", 240145))209+ }210+211+ @Test212+ fun `isExpectedApk rejects a downgrade`() {213+ // A signed APK with the right package name but a lower versionCode is214+ // still the wrong APK — refuse so a stale or partial cache can't slip215+ // through.216+ assertFalse(isExpectedApk("com.peachpasswords", 1L, "com.peachpasswords", 240145))217+ }218+219+ @Test220+ fun `isExpectedApk rejects null and blank package names`() {221+ assertFalse(isExpectedApk(null, 999L, "com.peachpasswords", 240145))222+ assertFalse(isExpectedApk("", 999L, "com.peachpasswords", 240145))110223 }111224 }