Skip invalid binary references during XML decoding

kotpass/src/main/kotlin/app/keemobile/kotpass/xml/BinaryReference.kt

@@ -8,18 +8,36 @@ import app.keemobile.kotpass.xml.FormatXml.Tags
 import org.redundent.kotlin.xml.Node
 import org.redundent.kotlin.xml.node
 
+/**
+ * Note that references with invalid IDs are skipped.
+ * This table represents how other clients handle this situation:
+ *
+ * | Client    | Behaviour                               |
+ * |-----------|-----------------------------------------|
+ * | KeePass   | Drops invalid attachments               |
+ * | KeePassXC | Keeps attachments as invalid references |
+ * | KeeWeb    | Drops invalid attachments               |
+ * | MacPass   | Drops invalid attachments               |
+ *
+ * @return BinaryReference or **null** when reference ID is invalid.
+ */
 internal fun unmarshalBinaryReference(
     context: XmlContext.Decode,
     node: Node
-): BinaryReference {
+): BinaryReference? {
     val id = node
         .firstOrNull(Tags.Entry.BinaryReferences.ItemValue)
         ?.get<String?>(FormatXml.Attributes.Ref)
         ?.toInt()
         ?: throw FormatError.InvalidXml("Invalid binary reference id.")
+    val hash = context
+        .binaries
+        .keys
+        .elementAtOrNull(id)
+        ?: return null
 
     return BinaryReference(
-        hash = context.binaries.keys.elementAt(id),
+        hash = hash,
         name = node
             .firstOrNull(Tags.Entry.BinaryReferences.ItemKey)
             ?.getText()

kotpass/src/main/kotlin/app/keemobile/kotpass/xml/Entry.kt

@@ -68,7 +68,7 @@ internal fun unmarshalEntry(
                         ?.forEach(tags::add)
                 }
                 Tags.Entry.BinaryReferences.TagName -> {
-                    binaries.add(unmarshalBinaryReference(context, childNode))
+                    unmarshalBinaryReference(context, childNode)?.let(binaries::add)
                 }
                 Tags.Entry.History -> {
                     history = unmarshalEntries(context, childNode).toMutableList()

kotpass/src/test/kotlin/app/keemobile/kotpass/models/EntrySpec.kt

@@ -2,13 +2,20 @@ package app.keemobile.kotpass.models
 
 import app.keemobile.kotpass.builders.buildEntry
 import app.keemobile.kotpass.constants.BasicField
+import app.keemobile.kotpass.cryptography.EncryptedValue
 import app.keemobile.kotpass.cryptography.EncryptionSaltGenerator
+import app.keemobile.kotpass.database.Credentials
+import app.keemobile.kotpass.database.modifiers.binaries
+import app.keemobile.kotpass.database.traverse
 import app.keemobile.kotpass.extensions.parseAsXml
 import app.keemobile.kotpass.resources.EntryRes
 import app.keemobile.kotpass.resources.TimeDataRes
+import app.keemobile.kotpass.resources.decodeFromResources
 import app.keemobile.kotpass.xml.unmarshalEntry
 import io.kotest.core.spec.style.DescribeSpec
+import io.kotest.matchers.collections.shouldBeEmpty
 import io.kotest.matchers.collections.shouldContainAll
+import io.kotest.matchers.maps.shouldBeEmpty
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import java.util.*
@@ -61,5 +68,19 @@ class EntrySpec : DescribeSpec({
             items shouldBe reversed
             entry1 shouldNotBe entry2
         }
+
+        it("Invalid binary references are skipped") {
+            val database = decodeFromResources(
+                path = "entry/invalid_references.kdbx",
+                credentials = Credentials.from(EncryptedValue.fromString("1"))
+            )
+
+            database.binaries.shouldBeEmpty()
+            database.traverse { element ->
+                if (element is Entry) {
+                    element.binaries.shouldBeEmpty()
+                }
+            }
+        }
     }
 })

kotpass/src/test/kotlin/app/keemobile/kotpass/resources/DecodeFromResources.kt

@@ -0,0 +1,23 @@
+package app.keemobile.kotpass.resources
+
+import app.keemobile.kotpass.database.Credentials
+import app.keemobile.kotpass.database.KeePassDatabase
+import app.keemobile.kotpass.database.decode
+import app.keemobile.kotpass.xml.DefaultXmlContentParser
+import app.keemobile.kotpass.xml.XmlContentParser
+
+internal fun decodeFromResources(
+    path: String,
+    credentials: Credentials,
+    validateHashes: Boolean = true,
+    contentParser: XmlContentParser = DefaultXmlContentParser
+) = ClassLoader
+    .getSystemResourceAsStream(path)!!
+    .use { inputStream ->
+        KeePassDatabase.decode(
+            inputStream = inputStream,
+            credentials = credentials,
+            validateHashes = validateHashes,
+            contentParser = contentParser
+        )
+    }