Fix v2 backup crash when group master key is invalid

app/src/main/java/org/thoughtcrime/securesms/backup/v2/ArchiveErrorCases.kt

@@ -179,6 +179,10 @@ object ExportSkips {
     return log(sentTimestamp, "Donation request not in Release Notes chat.")
   }
 
+  fun invalidGroupMasterKey(recipientId: Long): String {
+    return log(0, "Group had invalid master key (null or wrong length) for recipientId::$recipientId. Skipping group.")
+  }
+
   private fun log(sentTimestamp: Long, message: String): String {
     return "[SKIP][$sentTimestamp] $message"
   }

app/src/main/java/org/thoughtcrime/securesms/backup/v2/database/RecipientTableArchiveExtensions.kt

@@ -111,6 +111,7 @@ fun RecipientTable.getGroupsForBackup(selfAci: ServiceId.ACI): GroupArchiveExpor
     .where(
       """
       ${GroupTable.TABLE_NAME}.${GroupTable.V2_MASTER_KEY} IS NOT NULL AND
+      LENGTH(${GroupTable.TABLE_NAME}.${GroupTable.V2_MASTER_KEY}) = 32 AND
       ${GroupTable.TABLE_NAME}.${GroupTable.V2_REVISION} >= 0
       """
     )

app/src/main/java/org/thoughtcrime/securesms/backup/v2/exporters/GroupArchiveExporter.kt

@@ -9,12 +9,13 @@ import android.database.Cursor
 import okio.ByteString.Companion.toByteString
 import org.signal.archive.proto.Group
 import org.signal.core.models.ServiceId
+import org.signal.core.util.logging.Log
 import org.signal.core.util.requireBlob
 import org.signal.core.util.requireBoolean
 import org.signal.core.util.requireInt
 import org.signal.core.util.requireLong
-import org.signal.core.util.requireNonNullBlob
 import org.signal.core.util.requireString
+import org.thoughtcrime.securesms.backup.v2.ExportSkips
 import org.signal.storageservice.storage.protos.groups.AccessControl
 import org.signal.storageservice.storage.protos.groups.Member
 import org.signal.storageservice.storage.protos.groups.local.DecryptedBannedMember
@@ -36,27 +37,42 @@ import java.io.Closeable
  * Provides a nice iterable interface over a [RecipientTable] cursor, converting rows to [ArchiveRecipient]s.
  * Important: Because this is backed by a cursor, you must close it. It's recommended to use `.use()` or try-with-resources.
  */
-class GroupArchiveExporter(private val selfAci: ServiceId.ACI, private val cursor: Cursor) : Iterator<ArchiveRecipient>, Closeable {
+class GroupArchiveExporter(private val selfAci: ServiceId.ACI, private val cursor: Cursor) : Iterator<ArchiveRecipient?>, Closeable {
+
+  companion object {
+    private val TAG = Log.tag(GroupArchiveExporter::class.java)
+  }
 
   override fun hasNext(): Boolean {
     return cursor.count > 0 && !cursor.isLast
   }
 
-  override fun next(): ArchiveRecipient {
+  /**
+   * Returns the next [ArchiveRecipient], or `null` if the group's master key is invalid
+   * (null or wrong length) and should be skipped.
+   */
+  override fun next(): ArchiveRecipient? {
     if (!cursor.moveToNext()) {
       throw NoSuchElementException()
     }
 
+    val id = cursor.requireLong(RecipientTable.ID)
     val extras = RecipientTableCursorUtil.getExtras(cursor)
     val showAsStoryState: GroupTable.ShowAsStoryState = GroupTable.ShowAsStoryState.deserialize(cursor.requireInt(GroupTable.SHOW_AS_STORY_STATE))
 
     val isMember: Boolean = cursor.requireBoolean(GroupTable.IS_MEMBER)
     val decryptedGroup: DecryptedGroup? = cursor.requireBlob(GroupTable.V2_DECRYPTED_GROUP)?.let { DecryptedGroup.ADAPTER.decode(it) }
 
+    val masterKeyBytes: ByteArray? = cursor.requireBlob(GroupTable.V2_MASTER_KEY)
+    if (masterKeyBytes == null || masterKeyBytes.size != 32) {
+      Log.w(TAG, ExportSkips.invalidGroupMasterKey(id))
+      return null
+    }
+
     return ArchiveRecipient(
-      id = cursor.requireLong(RecipientTable.ID),
+      id = id,
       group = ArchiveGroup(
-        masterKey = cursor.requireNonNullBlob(GroupTable.V2_MASTER_KEY).toByteString(),
+        masterKey = masterKeyBytes.toByteString(),
         whitelisted = cursor.requireBoolean(RecipientTable.PROFILE_SHARING),
         blocked = cursor.requireBoolean(RecipientTable.BLOCKED),
         blockedAtTimestamp = cursor.requireLong(RecipientTable.BLOCKED_AT),

app/src/main/java/org/thoughtcrime/securesms/backup/v2/processor/RecipientArchiveProcessor.kt

@@ -88,7 +88,14 @@ object RecipientArchiveProcessor {
 
     db.recipientTable.getGroupsForBackup(selfAci).use { reader ->
       for (recipient in reader) {
-        exportState.recipientIds.add(recipient.id)
+        if (recipient == null) {
+          continue
+        }
+        val added = exportState.recipientIds.add(recipient.id)
+        if (!added) {
+          Log.w(TAG, ExportSkips.duplicateRecipientId(recipient.id))
+          continue
+        }
         exportState.groupRecipientIds.add(recipient.id)
         emitter.emit(Frame(recipient = recipient))
       }

app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt

@@ -616,7 +616,7 @@ class AttachmentTable(
     return readableDatabase
       .select(*PROJECTION_WITH_METADATA)
       .from(TABLE_NAME_WITH_METADTA)
-      .where("$DATA_HASH_END IS NOT NULL AND $DATA_FILE IS NOT NULL AND ${AttachmentMetadataTable.TABLE_NAME}.${AttachmentMetadataTable.LOCAL_BACKUP_KEY} IS NOT NULL")
+      .where("$DATA_HASH_END IS NOT NULL AND $DATA_FILE IS NOT NULL AND $DATA_RANDOM IS NOT NULL AND ${AttachmentMetadataTable.TABLE_NAME}.${AttachmentMetadataTable.LOCAL_BACKUP_KEY} IS NOT NULL")
       .orderBy("$TABLE_NAME.$ID DESC")
       .run()
       .readToList {
@@ -637,7 +637,7 @@ class AttachmentTable(
       .select(*PROJECTION_WITH_METADATA)
       .from("$TABLE_NAME_WITH_METADTA INNER JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.${MESSAGE_ID} = ${MessageTable.TABLE_NAME}.${MessageTable.ID}")
       .where(
-        "$DATA_HASH_END IS NOT NULL AND $DATA_FILE IS NOT NULL AND ${AttachmentMetadataTable.TABLE_NAME}.${AttachmentMetadataTable.LOCAL_BACKUP_KEY} IS NOT NULL" +
+        "$DATA_HASH_END IS NOT NULL AND $DATA_FILE IS NOT NULL AND $DATA_RANDOM IS NOT NULL AND ${AttachmentMetadataTable.TABLE_NAME}.${AttachmentMetadataTable.LOCAL_BACKUP_KEY} IS NOT NULL" +
           " AND ${MessageTable.TABLE_NAME}.${MessageTable.VIEW_ONCE} = 0" +
           " AND ${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} = 0"
       )