commit 11756da131e132018944ce1195d765592768d2ad
parent 7e8e9313fa55f2fc6d1e4d662c387af78e9fcb1f
Author: Michael Camilleri <[email protected]>
Date: Wed, 1 Jul 2026 20:50:30 +0900
Gate account push Decision adoption to the account zone
An accepted friend holds .readWrite on our private friend-<pairKey>
inbox zone, which lives in the private database alongside the account
zone. parseAccountPushAddressDecision and parseAccountPushSecretDecision
checked only the record type, name, kind, and payload, so a friend who
wrote a forged decision-account-pushSecret into that inbox could have it
adopted as this account's own HMAC secret. Adoption re-derives every
per-game push address and republishes it into every shared game's Player
record, giving an attacker denial of service and the ability to target
push addresses.
This commit ties both parse functions to the account zone. A new
isAccountZonePrivateDecision helper requires the record's zone name to
match accountZoneID and the fetch to have come from the private database
scope, mirroring the existing nickname case in applyDecisionRecord; the
scope is checked as well as the name because a peer can own a zone that
merely names itself like the account zone and share it in. Placing the
gate inside the parse functions keeps it a single source of truth that
neither the fetch path nor the send-conflict path can bypass.
The equal-version branch in adoptInboundPushSecret is left unchanged:
the zone gate already blocks every foreign record, and tightening that
branch would break legitimate convergence of a secret across the
account's own devices after a simultaneous mint.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
3 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift
@@ -216,10 +216,31 @@ enum RecordSerializer {
decisionRecordName(kind: accountDecisionKind, key: accountPushSecretDecisionKey)
}
- static func parseAccountPushAddressDecision(_ record: CKRecord) -> String? {
+ /// A push-address/secret Decision is authoritative only from *this
+ /// account's* zone in the private database. Match on zone name + private
+ /// scope, mirroring the `nickname` case in `applyDecisionRecord`: a friend
+ /// holds `.readWrite` on our private `friend-<pairKey>` inbox (also scope
+ /// 0), so without the zone-name check a forged `decision-account-pushSecret`
+ /// dropped there would be adopted as our own HMAC secret and re-derive every
+ /// per-game push address. An `==` on the full `zoneID` is unreliable — a
+ /// record fetched back from CloudKit often loses the
+ /// `CKCurrentUserDefaultName` owner placeholder — so match the name.
+ static func isAccountZonePrivateDecision(
+ _ record: CKRecord,
+ databaseScope: Int16
+ ) -> Bool {
+ record.recordID.zoneID.zoneName == accountZoneID.zoneName
+ && databaseScope == 0
+ }
+
+ static func parseAccountPushAddressDecision(
+ _ record: CKRecord,
+ databaseScope: Int16
+ ) -> String? {
guard record.recordType == "Decision",
record.recordID.recordName == accountPushAddressDecisionName,
(record["kind"] as? String) == accountDecisionKind,
+ isAccountZonePrivateDecision(record, databaseScope: databaseScope),
let address = record["payload"] as? String,
!address.isEmpty
else { return nil }
@@ -243,11 +264,13 @@ enum RecordSerializer {
}
static func parseAccountPushSecretDecision(
- _ record: CKRecord
+ _ record: CKRecord,
+ databaseScope: Int16
) -> (secret: String, version: Int64)? {
guard record.recordType == "Decision",
record.recordID.recordName == accountPushSecretDecisionName,
(record["kind"] as? String) == accountDecisionKind,
+ isAccountZonePrivateDecision(record, databaseScope: databaseScope),
let secret = record["payload"] as? String,
!secret.isEmpty
else { return nil }
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -1511,10 +1511,10 @@ actor SyncEngine {
effects.pings.append(ping)
}
case "Decision":
- if let address = RecordSerializer.parseAccountPushAddressDecision(record) {
+ if let address = RecordSerializer.parseAccountPushAddressDecision(record, databaseScope: scope) {
effects.accountPushAddresses.append(address)
}
- if let parsed = RecordSerializer.parseAccountPushSecretDecision(record) {
+ if let parsed = RecordSerializer.parseAccountPushSecretDecision(record, databaseScope: scope) {
effects.accountPushSecrets.append(parsed)
}
let wrote = RecordSerializer.applyDecisionRecord(
@@ -1886,10 +1886,11 @@ actor SyncEngine {
// device would keep deriving divergent per-game addresses.
settledDecisions.insert(failure.record.recordID)
if let serverRecord {
- if let address = RecordSerializer.parseAccountPushAddressDecision(serverRecord) {
+ let scope: Int16 = isPrivate ? 0 : 1
+ if let address = RecordSerializer.parseAccountPushAddressDecision(serverRecord, databaseScope: scope) {
accountAddresses.append(address)
}
- if let parsed = RecordSerializer.parseAccountPushSecretDecision(serverRecord) {
+ if let parsed = RecordSerializer.parseAccountPushSecretDecision(serverRecord, databaseScope: scope) {
accountSecrets.append(parsed)
}
}
diff --git a/Tests/Unit/RecordSerializerTests.swift b/Tests/Unit/RecordSerializerTests.swift
@@ -809,11 +809,45 @@ struct RecordSerializerTests {
zone: RecordSerializer.accountZoneID,
version: 5
)
- let parsed = RecordSerializer.parseAccountPushSecretDecision(record)
+ let parsed = RecordSerializer.parseAccountPushSecretDecision(record, databaseScope: 0)
#expect(parsed?.secret == "the-secret")
#expect(parsed?.version == 5)
}
+ @Test("parseAccountPushSecretDecision rejects a copy from a friend inbox zone")
+ func parseAccountPushSecretDecisionRejectsFriendZone() {
+ // A friend holds .readWrite on our private friend-<pairKey> inbox (also
+ // scope 0). A forged secret dropped there must never be adopted as our
+ // own HMAC secret — only the account zone counts.
+ let friendZone = CKRecordZone.ID(
+ zoneName: FriendZone.zoneName(pairKey: "pair-abc"),
+ ownerName: CKCurrentUserDefaultName
+ )
+ let record = RecordSerializer.decisionRecord(
+ kind: RecordSerializer.accountDecisionKind,
+ key: RecordSerializer.accountPushSecretDecisionKey,
+ payload: "forged-secret",
+ zone: friendZone,
+ version: 99
+ )
+ #expect(RecordSerializer.parseAccountPushSecretDecision(record, databaseScope: 0) == nil)
+ }
+
+ @Test("parseAccountPushSecretDecision rejects an account-named zone in the shared scope")
+ func parseAccountPushSecretDecisionRejectsSharedScope() {
+ // A peer can own a zone that *names* itself like our account zone and
+ // share it to us; it arrives via the shared DB (scope 1). The scope
+ // gate rejects it even though the zone name matches.
+ let record = RecordSerializer.decisionRecord(
+ kind: RecordSerializer.accountDecisionKind,
+ key: RecordSerializer.accountPushSecretDecisionKey,
+ payload: "forged-secret",
+ zone: RecordSerializer.accountZoneID,
+ version: 99
+ )
+ #expect(RecordSerializer.parseAccountPushSecretDecision(record, databaseScope: 1) == nil)
+ }
+
@Test("parseAccountPushSecretDecision treats a missing version as the base generation")
func parseAccountPushSecretDecisionDefaultsVersion() {
let record = RecordSerializer.decisionRecord(
@@ -822,7 +856,7 @@ struct RecordSerializerTests {
payload: "legacy-secret",
zone: RecordSerializer.accountZoneID
)
- let parsed = RecordSerializer.parseAccountPushSecretDecision(record)
+ let parsed = RecordSerializer.parseAccountPushSecretDecision(record, databaseScope: 0)
#expect(parsed?.version == RecordSerializer.decisionBaseVersion)
}
