From 417197f109a52dbdf85cfb0b1378b6937cd3b2c0 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:38:19 -0700 Subject: [PATCH] fix(mobile): reject coerced manifest scalars --- ...hybrid-webview-implementation-checklist.md | 9 ++++- ...-mobile-hybrid-webview-parity-inventory.md | 2 +- ...bile-hybrid-webview-single-pr-migration.md | 6 ++++ ...27-mobile-hybrid-webview-remaining-work.md | 5 ++- .../mobilewebshell/MobileWebPackageStore.kt | 34 ++++++++----------- .../MobileWebPackageStoreTest.kt | 30 ++++++++++++++++ .../MobileWebPackageStoreTests.swift | 34 +++++++++++++++++++ .../mobile-web/manifest-contract.test.ts | 26 ++++++++++++++ 8 files changed, 124 insertions(+), 22 deletions(-) diff --git a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md index 17b605d3c74..ca9dfad9b6f 100644 --- a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md +++ b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md @@ -1526,7 +1526,12 @@ copy. terminal-link tests reject unsupported targets. The exact Orca app route, native nested component, Android, physical-device, Release, broader live adversarial interaction, and independent review remain open. -- [ ] Fuzz manifest, chunks, asset paths, MIME types, CSP, and cache metadata. +- [~] Fuzz manifest, chunks, asset paths, MIME types, CSP, and cache metadata. + A mirrored TypeScript, Swift, and Kotlin scalar-type corpus rejects quoted + schema, bridge, total-byte, and asset-byte integers before staging. It found + and removed Android `JSONObject.optInt` coercion so native validation now + requires exact JSON integer/string types. Broader generated mutation, + chunk/path/MIME/CSP, and persisted-cache metadata fuzzing remains open. - [ ] Fuzz bridge envelopes, schemas, sizes, IDs, ordering, cancellation, and subscription lifecycle. - [ ] Attempt cross-host, cross-build, cross-workspace, and cross-session races. @@ -2546,4 +2551,6 @@ copy. | 2026-07-28 | Complete | Picker-interruption validation passes 568 mobile files / 3,371 tests with 2 expected skips, both typechecks and lints, changed-file formatting, 55 reliability gates, max-lines, package verification, and diff hygiene. RNW remains `9ed8c7f7d9be87c85b2431ece4eac3365a73e62bebf409846dea0ce72c9d1dde`: 49 assets, 9,280,463 raw bytes, and 2,684,481 gzip bytes. | | 2026-07-28 | Complete | Fresh exact-app iPhone 17 Pro / iOS 26.5 and Android API 36 arm64 Debug emulator runs loaded the active manifest-declared RNW script, rejected a mutated undeclared same-origin script path, and retained the hosted document. Both runs also passed network/navigation isolation; Android recorded zero sentinel observations and a clean bridge log. | | 2026-07-28 | Complete | Executable-isolation validation passes 568 mobile files / 3,373 tests with 2 expected skips and 1 focused file / 23 tests. Mobile and RNW typechecks/lints, full mobile and changed-file formatting, 55 reliability gates, max-lines, native Swift faults, 76-task Android module tests, package verification, and diff hygiene pass. RNW remains `9ed8c7f7…`: 49 assets, 9,280,463 raw bytes, and 2,684,481 gzip bytes. | +| 2026-07-28 | Finding | Android native manifest parsing accepted quoted numeric schema, bridge, total-byte, and asset-byte fields through coercive `JSONObject.optInt`, unlike the strict shared TypeScript schema and iOS parser. The Android store now requires exact scalar types, and the same five-case deterministic corpus passes in TypeScript, Swift, and Kotlin before any stage is created. | +| 2026-07-28 | Complete | Manifest scalar hardening passes 1 shared contract file / 20 tests, the native Swift fault executable, and the refreshed Android module suite across 76 Gradle tasks. Node typecheck, changed TypeScript lint/formatting, max-lines, and diff hygiene pass. No RNW package content changed. | | 2026-07-28 | Next | Complete the remaining parity inventory and cutover cleanup, then execute the physical-device, topology, security, performance, packaged-release, and App Store gates. | diff --git a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md index 73caf05933a..783edeb8ff6 100644 --- a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md +++ b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md @@ -487,7 +487,7 @@ IDs, and unrelated provider state never enter the page result. | Contract | Status | Source | | -------------------------------- | ----------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Multi-asset manifest v1 | Implemented and focused-test passing | `src/shared/mobile-web/manifest-contract.ts` | +| Multi-asset manifest v1 | Implemented and focused-test passing | `src/shared/mobile-web/manifest-contract.ts`; TypeScript, Swift, and Kotlin reject quoted numeric scalar confusion before staging | | Manifest resource bounds | Implemented, measured, and focused-test passing | 48 KiB chunks, 10 MiB/asset, 32 MiB/package, 256 assets; shared, Desktop, Swift, and Kotlin limits are regression-checked, and RNW has a 10 MiB CI budget | | Bridge envelope and capabilities | Implemented and focused-test passing | `src/shared/mobile-web/bridge-contract.ts`; bounded native-chat schemas and remaining operation payload schemas | | Terminal stream | Implemented and focused-test passing | `src/shared/mobile-web/terminal-stream-contract.ts`; broker adapter and real-stream validation remain | diff --git a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md index 3f72a064c93..a2805ec167b 100644 --- a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md +++ b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md @@ -2708,6 +2708,12 @@ Both runs also pass network/navigation isolation; Android records zero sentinel observations and no native bridge error. This does not replace physical-device, store-signed release, fuzz, or independent adversarial evidence. +A mirrored TypeScript, Swift, and Kotlin manifest corpus also rejects quoted +numeric schema, bridge, total-byte, and asset-byte fields before native staging. +The corpus found Android `JSONObject.optInt` coercion; the native parser now +requires exact JSON scalar types. Generated mutation and the remaining +chunk/path/MIME/CSP/cache corpus are still required. + ## App Store Gate App Review is a product gate. The PR must provide: diff --git a/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md b/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md index 89396cc0b97..d4d7d01aa82 100644 --- a/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md +++ b/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md @@ -201,7 +201,10 @@ cross-scope races, privacy/authorization audit, and independent review. release app on both platforms and complete independent live interaction testing. - [ ] Fuzz manifests, chunks, paths, MIME types, CSP, cache metadata, bridge - envelopes, limits, ordering, cancellation, and subscriptions. + envelopes, limits, ordering, cancellation, and subscriptions. The + five-case TypeScript/Swift/Kotlin quoted-numeric manifest corpus passes + after removing Android `JSONObject.optInt` coercion; generated mutation + and the other listed boundaries remain. - [ ] Attempt cross-host, cross-build, cross-workspace, cross-session, replay, reconnect, process-loss, and host-removal races. - [ ] Verify no credential or privileged host identity reaches URLs, DOM state, diff --git a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebPackageStore.kt b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebPackageStore.kt index 3d4fcc3de33..6937fd60c84 100644 --- a/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebPackageStore.kt +++ b/mobile/packages/expo-mobile-web-shell/android/src/main/java/expo/modules/mobilewebshell/MobileWebPackageStore.kt @@ -324,14 +324,14 @@ internal class MobileWebPackageStore internal constructor( require( manifest.keys().asSequence().toSet() == setOf("schemaVersion", "buildId", "bridge", "entrypoint", "totalBytes", "assets") && - manifest.optInt("schemaVersion", -1) == 1 + strictJsonInt(manifest, "schemaVersion") == 1 ) { "mobile_web_stage_manifest_invalid" } val bridge = manifest.optJSONObject("bridge") ?: throw IllegalArgumentException("mobile_web_stage_manifest_invalid") - val bridgeMinimum = bridge.optInt("minimum", -1) - val bridgeTestedThrough = bridge.optInt("testedThrough", -1) - val entrypoint = manifest.optString("entrypoint") - val declaredTotalBytes = manifest.optInt("totalBytes", -1) + val bridgeMinimum = strictJsonInt(bridge, "minimum") ?: -1 + val bridgeTestedThrough = strictJsonInt(bridge, "testedThrough") ?: -1 + val entrypoint = strictJsonString(manifest, "entrypoint") ?: "" + val declaredTotalBytes = strictJsonInt(manifest, "totalBytes") ?: -1 require( bridge.keys().asSequence().toSet() == setOf("minimum", "testedThrough") && bridgeMinimum > 0 && @@ -340,7 +340,7 @@ internal class MobileWebPackageStore internal constructor( isSafeAssetPath(entrypoint) && declaredTotalBytes in 1..(32 * 1024 * 1024) ) { "mobile_web_stage_manifest_invalid" } - val buildId = manifest.optString("buildId") + val buildId = strictJsonString(manifest, "buildId") ?: "" require( SHA256_PATTERN.matches(buildId) && sha256Hex(canonicalManifestJson.toByteArray(Charsets.UTF_8)) == buildId @@ -361,11 +361,11 @@ internal class MobileWebPackageStore internal constructor( value.keys().asSequence().toSet() == setOf("path", "sha256", "byteLength", "contentType", "role") ) { "mobile_web_stage_manifest_invalid" } - val path = value.optString("path") - val hash = value.optString("sha256") - val length = value.optInt("byteLength", -1) - val contentType = value.optString("contentType") - val role = value.optString("role") + val path = strictJsonString(value, "path") ?: "" + val hash = strictJsonString(value, "sha256") ?: "" + val length = strictJsonInt(value, "byteLength") ?: -1 + val contentType = strictJsonString(value, "contentType") ?: "" + val role = strictJsonString(value, "role") ?: "" require( isSafeAssetPath(path) && SHA256_PATTERN.matches(hash) && @@ -382,9 +382,7 @@ internal class MobileWebPackageStore internal constructor( require( totalBytes == declaredTotalBytes && documentCount == 1 && - assetValues.asObjectSequence().any { - it.optString("path") == entrypoint && it.optString("role") == "document" - } + assets[entrypoint]?.role == "document" ) { "mobile_web_stage_manifest_invalid" } return MobileWebManifestRecord( buildId, @@ -638,11 +636,9 @@ private fun isValidAssetMetadata( return fileHash == hash && expectedMetadata.first == contentType && expectedMetadata.second == role } -private fun JSONArray.asObjectSequence(): Sequence = sequence { - for (index in 0 until length()) { - optJSONObject(index)?.let { yield(it) } - } -} +private fun strictJsonInt(value: JSONObject, key: String): Int? = value.opt(key) as? Int + +private fun strictJsonString(value: JSONObject, key: String): String? = value.opt(key) as? String private fun sha256Hex(bytes: ByteArray): String = MessageDigest.getInstance("SHA-256").digest(bytes).joinToString("") { "%02x".format(it) } diff --git a/mobile/packages/expo-mobile-web-shell/android/src/test/java/expo/modules/mobilewebshell/MobileWebPackageStoreTest.kt b/mobile/packages/expo-mobile-web-shell/android/src/test/java/expo/modules/mobilewebshell/MobileWebPackageStoreTest.kt index 28da84eea59..961725ed886 100644 --- a/mobile/packages/expo-mobile-web-shell/android/src/test/java/expo/modules/mobilewebshell/MobileWebPackageStoreTest.kt +++ b/mobile/packages/expo-mobile-web-shell/android/src/test/java/expo/modules/mobilewebshell/MobileWebPackageStoreTest.kt @@ -59,6 +59,36 @@ class MobileWebPackageStoreTest { assertFalse(root.walkTopDown().any { it.name == "staging" && it.listFiles()?.isNotEmpty() == true }) } + @Test + fun rejectsQuotedNumericManifestFieldsBeforeCreatingAStage() { + val root = temporary.newFolder() + val store = MobileWebPackageStore(root) + val valid = packageFixture() + val invalid = listOf( + packageFixture { _, manifest -> manifest.put("schemaVersion", "1") }, + packageFixture { _, manifest -> + manifest.getJSONObject("bridge").put("minimum", "1") + }, + packageFixture { _, manifest -> + manifest.getJSONObject("bridge").put("testedThrough", "1") + }, + packageFixture { _, manifest -> manifest.put("totalBytes", valid.bytes.size.toString()) }, + packageFixture(mutateAsset = { asset -> + asset.put("byteLength", valid.bytes.size.toString()) + }) + ) + + invalid.forEach { fixture -> + val error = assertThrows(IllegalArgumentException::class.java) { + store.beginStage("paired-host", fixture.manifest, fixture.canonical) + } + assertEquals("mobile_web_stage_manifest_invalid", error.message) + } + assertFalse( + root.walkTopDown().any { it.name == "staging" && it.listFiles()?.isNotEmpty() == true } + ) + } + @Test fun deletesAnInterruptedStageWhenTheStoreRestarts() { val root = temporary.newFolder() diff --git a/mobile/packages/expo-mobile-web-shell/ios-tests/MobileWebPackageStoreTests.swift b/mobile/packages/expo-mobile-web-shell/ios-tests/MobileWebPackageStoreTests.swift index 1e46114c2e1..afe20d09f41 100644 --- a/mobile/packages/expo-mobile-web-shell/ios-tests/MobileWebPackageStoreTests.swift +++ b/mobile/packages/expo-mobile-web-shell/ios-tests/MobileWebPackageStoreTests.swift @@ -13,6 +13,7 @@ enum MobileWebPackageStoreTests { try stagesAndReadsExactGeneration(root: root.appendingPathComponent("verified")) try rejectsMalformedManifests(root: root.appendingPathComponent("manifests")) + try rejectsQuotedNumericManifestFields(root: root.appendingPathComponent("scalar-types")) try deletesInterruptedStage(root: root.appendingPathComponent("interrupted")) try rejectsIncompleteAndCorruptGeneration(root: root.appendingPathComponent("corrupt")) try activatesAndRecoversPreviousGeneration(root: root.appendingPathComponent("rollback")) @@ -86,6 +87,39 @@ enum MobileWebPackageStoreTests { } } + private static func rejectsQuotedNumericManifestFields(root: URL) throws { + let store = MobileWebPackageStore(cacheRoot: root) + let valid = try packageFixture() + let invalid = [ + try packageFixture { $0["schemaVersion"] = "1" }, + try packageFixture { manifest in + var bridge = manifest["bridge"] as! [String: Any] + bridge["minimum"] = "1" + manifest["bridge"] = bridge + }, + try packageFixture { manifest in + var bridge = manifest["bridge"] as! [String: Any] + bridge["testedThrough"] = "1" + manifest["bridge"] = bridge + }, + try packageFixture { $0["totalBytes"] = String(valid.bytes.count) }, + try packageFixture { manifest in + mutateAsset(&manifest) { $0["byteLength"] = String(valid.bytes.count) } + }, + ] + for fixture in invalid { + precondition( + throwsError { + _ = try store.beginStage( + hostIdentity: "paired-host", + manifestJson: fixture.manifest, + canonicalManifestJson: fixture.canonical + ) + } + ) + } + } + private static func deletesInterruptedStage(root: URL) throws { let first = MobileWebPackageStore(cacheRoot: root) let fixture = try packageFixture() diff --git a/src/shared/mobile-web/manifest-contract.test.ts b/src/shared/mobile-web/manifest-contract.test.ts index 4ab04473944..6d7043ea6be 100644 --- a/src/shared/mobile-web/manifest-contract.test.ts +++ b/src/shared/mobile-web/manifest-contract.test.ts @@ -103,6 +103,32 @@ describe('mobile web manifest contract', () => { expect(MobileWebManifestSchema.safeParse(wrongTotal).success).toBe(false) }) + it.each([ + ['schemaVersion', (manifest: Record) => (manifest.schemaVersion = '1')], + [ + 'bridge.minimum', + (manifest: Record) => + ((manifest.bridge as Record).minimum = '1') + ], + [ + 'bridge.testedThrough', + (manifest: Record) => + ((manifest.bridge as Record).testedThrough = '2') + ], + ['totalBytes', (manifest: Record) => (manifest.totalBytes = '60')], + [ + 'assets.byteLength', + (manifest: Record) => { + const assets = manifest.assets as Record[] + assets[0]!.byteLength = '20' + } + ] + ])('rejects quoted numeric field %s', (_field, mutate) => { + const manifest = validManifest() as unknown as Record + mutate(manifest) + expect(MobileWebManifestSchema.safeParse(manifest).success).toBe(false) + }) + it('enforces bridge, per-asset, and file-count bounds', () => { const invalidBridge = validManifest() invalidBridge.bridge = { minimum: 3, testedThrough: 2 }