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
3 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 −0
app/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")

view file

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() {

view file

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 }

view file