MXKeyBackup: Add sanity checks to avoid crashes

element-hq/element-ios/issues/4113 Avoid crashes and log what is wrong
matrix-org · Mar 24, 2021 · 181b860 · 181b860
1 parent 6a937c0
commit 181b860
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 3 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -16,6 +16,7 @@ Changes to be released in next version
  * MXDeviceList: Fix memory leak.
  * MXDeviceListOperation: Fix memory leak.
  * MXRoomState/MXRoomMembers: Fix memory leak and copying.
+ * MXKeyBackup: Add sanity checks to avoid crashes (vector-im/element-ios/issues/4113).
 
 ⚠️ API Changes
  * 

diff --git a/MatrixSDK/Crypto/KeyBackup/MXKeyBackup.m b/MatrixSDK/Crypto/KeyBackup/MXKeyBackup.m
@@ -275,11 +275,14 @@ - (void)sendKeyBackup
     {
         MXKeyBackupData *keyBackupData = [self encryptGroupSession:session];
 
-        if (!roomsKeyBackup[session.roomId])
+        if (keyBackupData)
         {
-            roomsKeyBackup[session.roomId] = [NSMutableDictionary dictionary];
+            if (!roomsKeyBackup[session.roomId])
+            {
+                roomsKeyBackup[session.roomId] = [NSMutableDictionary dictionary];
+            }
+            roomsKeyBackup[session.roomId][session.session.sessionIdentifier] = keyBackupData;
         }
-        roomsKeyBackup[session.roomId][session.session.sessionIdentifier] = keyBackupData;
     }
 
     NSLog(@"[MXKeyBackup] sendKeyBackup: 3 - Finalising data to send");
@@ -1526,10 +1529,21 @@ - (MXKeyBackupData*)encryptGroupSession:(MXOlmInboundGroupSession*)session
 {
     // Gather information for each key
     MXDeviceInfo *device = [crypto.deviceList deviceWithIdentityKey:session.senderKey andAlgorithm:kMXCryptoMegolmAlgorithm];
+    if (!device)
+    {
+        NSLog(@"[MXKeyBackup] encryptGroupSession: Error: Cannot find device for %@", session.senderKey);
+        return nil;
+    }
 
     // Build the m.megolm_backup.v1.curve25519-aes-sha2 data as defined at
     // https://github.com/uhoreg/matrix-doc/blob/e2e_backup/proposals/1219-storing-megolm-keys-serverside.md#mmegolm_backupv1curve25519-aes-sha2-key-format
     MXMegolmSessionData *sessionData = session.exportSessionData;
+    if (![self checkMXMegolmSessionData:sessionData])
+    {
+        NSLog(@"[MXKeyBackup] encryptGroupSession: Error: Invalid MXMegolmSessionData for %@", session.senderKey);
+        return nil;
+    }
+
     NSDictionary *sessionBackupData = @{
                                         @"algorithm": sessionData.algorithm,
                                         @"sender_key": sessionData.senderKey,
@@ -1538,6 +1552,11 @@ - (MXKeyBackupData*)encryptGroupSession:(MXOlmInboundGroupSession*)session
                                         @"session_key": sessionData.sessionKey
                                         };
     OLMPkMessage *encryptedSessionBackupData = [_backupKey encryptMessage:[MXTools serialiseJSONObject:sessionBackupData] error:nil];
+    if (![self checkOLMPkMessage:encryptedSessionBackupData])
+    {
+        NSLog(@"[MXKeyBackup] encryptGroupSession: Error: Invalid OLMPkMessage for %@", session.senderKey);
+        return nil;
+    }
 
     // Build backup data for that key
     MXKeyBackupData *keyBackupData = [MXKeyBackupData new];
@@ -1553,6 +1572,55 @@ - (MXKeyBackupData*)encryptGroupSession:(MXOlmInboundGroupSession*)session
     return keyBackupData;
 }
 
+// Sanity checks on MXMegolmSessionData
+- (BOOL)checkMXMegolmSessionData:(MXMegolmSessionData*)sessionData
+{
+    if (!sessionData.algorithm)
+    {
+        NSLog(@"[MXKeyBackup] checkMXMegolmSessionData: missing algorithm");
+        return NO;
+    }
+    if (!sessionData.senderKey)
+    {
+        NSLog(@"[MXKeyBackup] checkMXMegolmSessionData: missing senderKey");
+        return NO;
+    }
+    if (!sessionData.senderClaimedKeys)
+    {
+        NSLog(@"[MXKeyBackup] checkMXMegolmSessionData: missing senderClaimedKeys");
+        return NO;
+    }
+    if (!sessionData.sessionKey)
+    {
+        NSLog(@"[MXKeyBackup] checkMXMegolmSessionData: missing sessionKey");
+        return NO;
+    }
+
+    return YES;
+}
+
+// Sanity checks on OLMPkMessage
+- (BOOL)checkOLMPkMessage:(OLMPkMessage*)message
+{
+    if (!message.ciphertext)
+    {
+        NSLog(@"[MXKeyBackup] checkOLMPkMessage: missing ciphertext");
+        return NO;
+    }
+    if (!message.mac)
+    {
+        NSLog(@"[MXKeyBackup] checkOLMPkMessage: missing mac");
+        return NO;
+    }
+    if (!message.ephemeralKey)
+    {
+        NSLog(@"[MXKeyBackup] checkOLMPkMessage: missing ephemeralKey");
+        return NO;
+    }
+
+    return YES;
+}
+
 - (MXMegolmSessionData*)decryptKeyBackupData:(MXKeyBackupData*)keyBackupData forSession:(NSString*)sessionId inRoom:(NSString*)roomId withPkDecryption:(OLMPkDecryption*)decryption
 {
     MXMegolmSessionData *sessionData;