From d1af88617bcee9296d37e96533d86055d61de1c9 Mon Sep 17 00:00:00 2001 From: areebahmeddd Date: Fri, 31 Jul 2026 08:40:06 +0530 Subject: [PATCH] prevent changing creator in groups --- bitchat/Services/Groups/GroupStore.swift | 22 +++++++++- bitchatTests/Services/GroupStoreTests.swift | 47 +++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/bitchat/Services/Groups/GroupStore.swift b/bitchat/Services/Groups/GroupStore.swift index 3a45dcb7..a8bdca6d 100644 --- a/bitchat/Services/Groups/GroupStore.swift +++ b/bitchat/Services/Groups/GroupStore.swift @@ -68,7 +68,8 @@ final class GroupStore: ObservableObject { } /// Inserts or replaces a group and its current key. Rejects rosters over - /// the hard cap or groups whose creator is missing from the roster. + /// the hard cap, groups whose creator is missing from the roster, and any + /// attempt to change the creator of a group we already hold. @discardableResult func upsert(_ group: BitchatGroup, key: Data) -> Bool { guard group.groupID.count == BitchatGroup.groupIDLength, @@ -76,6 +77,25 @@ final class GroupStore: ObservableObject { !group.members.isEmpty, group.members.count <= BitchatGroup.maxMembers, group.creator != nil else { return false } + // A group keeps the creator it was created with. + // + // `applyGroupState` checks that the sender is the creator the state + // NAMES, that the creator signature verifies, that we are still in the + // roster, and that the epoch does not regress — but nothing compares the + // creator against the group we already hold. Each check passes for an + // attacker who simply names themselves creator of an existing groupID at + // a higher epoch and signs with their own key, and the roster and key are + // then replaced wholesale. Members would seal their next message under a + // key the attacker holds, while the real group silently stops seeing + // them; nothing in the UI changes. + // + // Pinning here rather than in `applyGroupState` because this is the one + // point every write goes through, local and remote alike. + if let existing = groups.first(where: { $0.groupID == group.groupID }), + existing.creatorFingerprint != group.creatorFingerprint { + SecureLogger.warning("Refusing group state that changes the creator of an existing group", category: .security) + return false + } guard keychain.saveIdentityKey(key, forKey: Self.keychainKey(for: group.groupID)) else { SecureLogger.error("Failed to store group key in keychain", category: .security) return false diff --git a/bitchatTests/Services/GroupStoreTests.swift b/bitchatTests/Services/GroupStoreTests.swift index adf04806..a8ca7cce 100644 --- a/bitchatTests/Services/GroupStoreTests.swift +++ b/bitchatTests/Services/GroupStoreTests.swift @@ -81,6 +81,53 @@ struct GroupStoreTests { #expect(store.group(withID: group.groupID)?.members == [creator]) } + // MARK: - Creator pinning + + @Test func upsertRefusesToChangeTheCreatorOfAnExistingGroup() throws { + let store = GroupStore(keychain: MockKeychain(), persistsToDisk: false) + let creator = makeMember(seed: 0xC1, nickname: "creator") + let me = makeMember(seed: 0x0E, nickname: "me") + let attacker = makeMember(seed: 0xEE, nickname: "attacker") + let group = try #require(store.createGroup(named: "trip", creator: creator)) + #expect(store.updateRoster(groupID: group.groupID, members: [creator, me]) != nil) + let realKey = try #require(store.key(forGroupID: group.groupID)) + + // Same groupID, same name, higher epoch, attacker as creator — every + // other check in applyGroupState passes for this. + let hijack = BitchatGroup( + groupID: group.groupID, + name: group.name, + epoch: group.epoch + 1, + members: [attacker, me], + creatorFingerprint: attacker.fingerprint + ) + + #expect(!store.upsert(hijack, key: Data(repeating: 0xAB, count: 32))) + + let stored = try #require(store.group(withID: group.groupID)) + #expect(stored.creatorFingerprint == creator.fingerprint) + #expect(stored.epoch == group.epoch) + #expect(store.key(forGroupID: group.groupID) == realKey) + } + + @Test func upsertStillAcceptsANewEpochFromTheSameCreator() throws { + // The other side of the boundary: pinning the creator must not stop the + // real creator rotating the key, or removing a member breaks. + let store = GroupStore(keychain: MockKeychain(), persistsToDisk: false) + let creator = makeMember(seed: 0xC1, nickname: "creator") + let me = makeMember(seed: 0x0E, nickname: "me") + let group = try #require(store.createGroup(named: "trip", creator: creator)) + + var rotated = group + rotated.epoch = group.epoch + 1 + rotated.members = [creator, me] + let newKey = Data(repeating: 0x5A, count: 32) + + #expect(store.upsert(rotated, key: newKey)) + #expect(store.group(withID: group.groupID)?.epoch == group.epoch + 1) + #expect(store.key(forGroupID: group.groupID) == newKey) + } + // MARK: - Rotation @Test func rotateKeyBumpsEpochAndReplacesKey() throws {