From a5306f85b7b7094ebc1f61362c6661d1ce81cc4e Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Fri, 7 Jul 2023 15:19:13 +1000 Subject: [PATCH] Added in a little defensive coding for config message processing Updated the config 'pendingChanges' to use the readonly version of the conf (no use blocking access) Added code to throw and log when the config processing exceeds 50000 loops (ie. infinite loop protection) --- .../Config Handling/SessionUtil+Contacts.swift | 7 +++++-- .../SessionUtil+ConvoInfoVolatile.swift | 12 ++++++------ .../Config Handling/SessionUtil+Shared.swift | 14 ++++++++++---- .../Config Handling/SessionUtil+UserGroups.swift | 3 +++ SessionMessagingKit/SessionUtil/SessionUtil.swift | 10 ++++------ .../SessionUtil/SessionUtilError.swift | 1 + 6 files changed, 29 insertions(+), 18 deletions(-) diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift index 9487b7b68..1d142893d 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Contacts.swift @@ -40,7 +40,7 @@ internal extension SessionUtil { // The current users contact data is handled separately so exclude it if it's present (as that's // actually a bug) let userPublicKey: String = getUserHexEncodedPublicKey(db) - let targetContactData: [String: ContactData] = extractContacts( + let targetContactData: [String: ContactData] = try extractContacts( from: conf, latestConfigSentTimestampMs: latestConfigSentTimestampMs ).filter { $0.key != userPublicKey } @@ -540,12 +540,15 @@ private extension SessionUtil { static func extractContacts( from conf: UnsafeMutablePointer?, latestConfigSentTimestampMs: Int64 - ) -> [String: ContactData] { + ) throws -> [String: ContactData] { + var infiniteLoopGuard: Int = 0 var result: [String: ContactData] = [:] var contact: contacts_contact = contacts_contact() let contactIterator: UnsafeMutablePointer = contacts_iterator_new(conf) while !contacts_iterator_done(contactIterator, &contact) { + try SessionUtil.checkLoopLimitReached(&infiniteLoopGuard, for: .contacts) + let contactId: String = String(cString: withUnsafeBytes(of: contact.session_id) { [UInt8]($0) } .map { CChar($0) } .nullTerminated() diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift index 677f5bd81..c504a0c35 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+ConvoInfoVolatile.swift @@ -23,7 +23,7 @@ internal extension SessionUtil { guard conf != nil else { throw SessionUtilError.nilConfigObject } // Get the volatile thread info from the conf and local conversations - let volatileThreadInfo: [VolatileThreadInfo] = extractConvoVolatileInfo(from: conf) + let volatileThreadInfo: [VolatileThreadInfo] = try extractConvoVolatileInfo(from: conf) let localVolatileThreadInfo: [String: VolatileThreadInfo] = VolatileThreadInfo.fetchAll(db) .reduce(into: [:]) { result, next in result[next.threadId] = next } @@ -314,10 +314,7 @@ public extension SessionUtil { openGroup: OpenGroup? ) -> Bool { return SessionUtil - .config( - for: .convoInfoVolatile, - publicKey: userPublicKey - ) + .config(for: .convoInfoVolatile, publicKey: userPublicKey) .wrappedValue .map { conf in switch threadVariant { @@ -512,7 +509,8 @@ public extension SessionUtil { internal static func extractConvoVolatileInfo( from conf: UnsafeMutablePointer? - ) -> [VolatileThreadInfo] { + ) throws -> [VolatileThreadInfo] { + var infiniteLoopGuard: Int = 0 var result: [VolatileThreadInfo] = [] var oneToOne: convo_info_volatile_1to1 = convo_info_volatile_1to1() var community: convo_info_volatile_community = convo_info_volatile_community() @@ -520,6 +518,8 @@ public extension SessionUtil { let convoIterator: OpaquePointer = convo_info_volatile_iterator_new(conf) while !convo_info_volatile_iterator_done(convoIterator) { + try SessionUtil.checkLoopLimitReached(&infiniteLoopGuard, for: .convoInfoVolatile) + if convo_info_volatile_it_is_1to1(convoIterator, &oneToOne) { result.append( VolatileThreadInfo( diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Shared.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Shared.swift index 6c23a7b7d..909ea9ce7 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Shared.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+Shared.swift @@ -59,10 +59,7 @@ internal extension SessionUtil { do { needsPush = try SessionUtil - .config( - for: variant, - publicKey: publicKey - ) + .config(for: variant, publicKey: publicKey) .mutate { conf in guard conf != nil else { throw SessionUtilError.nilConfigObject } @@ -332,6 +329,15 @@ internal extension SessionUtil { // Ensure the change occurred after the last config message was handled (minus the buffer period) return (changeTimestampMs >= (configDumpTimestampMs - Int64(SessionUtil.configChangeBufferPeriod * 1000))) } + + static func checkLoopLimitReached(_ loopCounter: inout Int, for variant: ConfigDump.Variant, maxLoopCount: Int = 50000) throws { + loopCounter += 1 + + guard loopCounter < maxLoopCount else { + SNLog("[libSession] Got stuck in infinite loop processing '\(variant.configMessageKind.description)' data") + throw SessionUtilError.processingLoopLimitReached + } + } } // MARK: - External Outgoing Changes diff --git a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserGroups.swift b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserGroups.swift index ba9210a74..a10e617be 100644 --- a/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserGroups.swift +++ b/SessionMessagingKit/SessionUtil/Config Handling/SessionUtil+UserGroups.swift @@ -34,6 +34,7 @@ internal extension SessionUtil { guard mergeNeedsDump else { return } guard conf != nil else { throw SessionUtilError.nilConfigObject } + var infiniteLoopGuard: Int = 0 var communities: [PrioritisedData] = [] var legacyGroups: [LegacyGroupInfo] = [] var community: ugroups_community_info = ugroups_community_info() @@ -41,6 +42,8 @@ internal extension SessionUtil { let groupsIterator: OpaquePointer = user_groups_iterator_new(conf) while !user_groups_iterator_done(groupsIterator) { + try SessionUtil.checkLoopLimitReached(&infiniteLoopGuard, for: .userGroups) + if user_groups_it_is_community(groupsIterator, &community) { let server: String = String(libSessionVal: community.base_url) let roomToken: String = String(libSessionVal: community.room) diff --git a/SessionMessagingKit/SessionUtil/SessionUtil.swift b/SessionMessagingKit/SessionUtil/SessionUtil.swift index 204068860..62b77c9fe 100644 --- a/SessionMessagingKit/SessionUtil/SessionUtil.swift +++ b/SessionMessagingKit/SessionUtil/SessionUtil.swift @@ -314,9 +314,10 @@ public enum SessionUtil { .compactMap { variant -> OutgoingConfResult? in try SessionUtil .config(for: variant, publicKey: publicKey) - .mutate { conf in + .wrappedValue + .map { conf in // Check if the config needs to be pushed - guard conf != nil && config_needs_push(conf) else { return nil } + guard config_needs_push(conf) else { return nil } var cPushData: UnsafeMutablePointer! let configCountInfo: String = { @@ -375,10 +376,7 @@ public enum SessionUtil { publicKey: String ) -> ConfigDump? { return SessionUtil - .config( - for: message.kind.configDumpVariant, - publicKey: publicKey - ) + .config(for: message.kind.configDumpVariant, publicKey: publicKey) .mutate { conf in guard conf != nil else { return nil } diff --git a/SessionMessagingKit/SessionUtil/SessionUtilError.swift b/SessionMessagingKit/SessionUtil/SessionUtilError.swift index 1c3cd4d9e..42da99da5 100644 --- a/SessionMessagingKit/SessionUtil/SessionUtilError.swift +++ b/SessionMessagingKit/SessionUtil/SessionUtilError.swift @@ -7,4 +7,5 @@ public enum SessionUtilError: Error { case nilConfigObject case userDoesNotExist case getOrConstructFailedUnexpectedly + case processingLoopLimitReached }