From c21dda181718be96dda232fce20169a1f03c2c82 Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Fri, 31 Jul 2026 14:06:39 +0300 Subject: [PATCH 1/7] wip --- ...ed-plaintext-unsigned-html-alternative.eml | 40 +++++ .../email/ui/MessageDetailsFlowTest.kt | 34 ++++ .../flowcrypt/email/security/pgp/PgpMsg.kt | 146 +++++++++++------- .../security/pgp/ProcessMimeMessageTest.kt | 33 ++++ ...ed-plaintext-unsigned-html-alternative.eml | 40 +++++ .../denbond7@flowcrypt.test_pub_primary.asc | 13 ++ 6 files changed, 251 insertions(+), 55 deletions(-) create mode 100644 FlowCrypt/src/androidTest/assets/messages/mime/signed-plaintext-unsigned-html-alternative.eml create mode 100644 FlowCrypt/src/test/resources/mime/signed-plaintext-unsigned-html-alternative.eml create mode 100644 FlowCrypt/src/test/resources/pgp/keys/denbond7@flowcrypt.test_pub_primary.asc diff --git a/FlowCrypt/src/androidTest/assets/messages/mime/signed-plaintext-unsigned-html-alternative.eml b/FlowCrypt/src/androidTest/assets/messages/mime/signed-plaintext-unsigned-html-alternative.eml new file mode 100644 index 0000000000..f44715e53f --- /dev/null +++ b/FlowCrypt/src/androidTest/assets/messages/mime/signed-plaintext-unsigned-html-alternative.eml @@ -0,0 +1,40 @@ +Return-Path: +Delivered-To: default@flowcrypt.test +Message-ID: +Date: Sat, 20 Jun 2026 12:00:00 +0000 +MIME-Version: 1.0 +To: default@flowcrypt.test +From: DenBond7 +Subject: Signed plaintext alternative, unsigned HTML alternative +Content-Type: multipart/alternative; boundary="fc-alt-signed-plain-unsigned-html" + +--fc-alt-signed-plain-unsigned-html +Content-Type: text/plain; charset=UTF-8; format=flowed +Content-Transfer-Encoding: 7bit + +-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA512 + +It's a cleartext signed message +-----BEGIN PGP SIGNATURE----- +Version: PGPainless + +iHUEARYKACcFAmLs1kUJEMMgic1q+NbOFiEEwWQo1gHLOTeJz2HUwyCJzWr41s4A +AG8wAP44Q3Zzr7sDM9V34A3CdDxMjUFSJcOsPZUuUi1kIZzv6wD/cxdthjQpC7uN +Hwvo1OGuQBE7TENCj1cW4P9AdOFxSAQ= +=McWq +-----END PGP SIGNATURE----- + +--fc-alt-signed-plain-unsigned-html +Content-Type: text/html; charset=UTF-8 +Content-Transfer-Encoding: 7bit + + + +

Unsigned HTML rendered by the message view

+

This visible HTML is not covered by the PGP signature in the text/plain alternative.

+

Example impact text: please approve the new payment destination ATTACKER-ACCOUNT.

+ + + +--fc-alt-signed-plain-unsigned-html-- diff --git a/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt b/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt index 7e5eff780d..c15c93b7f8 100644 --- a/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt +++ b/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt @@ -997,6 +997,40 @@ class MessageDetailsFlowTest : BaseMessageDetailsFlowTest() { ) } + @Test + fun testSignedPlainTextDoesNotMarkDisplayedUnsignedHtmlAsSigned() { + PrivateKeysManager.savePubKeyToDatabase("pgp/denbond7@flowcrypt.test_pub_primary.asc") + + val msgInfo = getMsgInfo( + path = "messages/info/standard_msg_info_plaintext.json", + mimeMsgPath = "messages/mime/signed-plaintext-unsigned-html-alternative.eml", + accountEntity = addAccountToDatabaseRule.accountEntityWithDecryptedInfo + ) + baseCheck(msgInfo, checkWebContent = false) + + onWebView(withId(R.id.emailWebView)).forceJavascriptEnabled() + onWebView(withId(R.id.emailWebView)) + .check( + webContent( + elementByXPath( + "/html/body", + withTextContent( + allOf( + containsString("ATTACKER-ACCOUNT"), + not(containsString("It's a cleartext signed message")) + ) + ) + ) + ) + ) + + testPgpBadges( + 2, + PgpBadgeListAdapter.PgpBadge.Type.NOT_ENCRYPTED, + PgpBadgeListAdapter.PgpBadge.Type.NOT_SIGNED + ) + } + @Test fun testSignatureVerificationCleartextOnlySignedPartially() { PrivateKeysManager.savePubKeyToDatabase("pgp/denbond7@flowcrypt.test_pub_primary.asc") diff --git a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt index 1986be71a9..993413d438 100644 --- a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt +++ b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt @@ -836,50 +836,57 @@ object PgpMsg { val verifiedSignatures = mutableListOf() val keyIdOfSigningKeys = mutableSetOf() - for (block in msgBlocks) { - // We don't need Base64 correction here, fromAttachment() does this for us - // We also seem to don't need to make correction between raw and utf8 - // But I'd prefer MsgBlock.content to be ByteArray - // So, at least meanwhile, not porting this: - // block.content = isContentBlock(block.type) - // ? block.content.toUtfStr() : block.content.toRawBytesStr(); - - filterBlocksViaTree(listOf(block)) { innerBlock -> - innerBlock.type in MsgBlock.Type.SIGNED_BLOCK_TYPES - }.forEach { pgpBlock -> - analyzeBlockForPgp(pgpBlock) { hasEncryptedContent, - hasSignedContent, - hasInvalidSignatures, - keyIdsOfSigningKeys, - verifiedSignaturesList -> - if (!isEncrypted) { - isEncrypted = hasEncryptedContent - } + filterBlocksViaTree(msgBlocks.toList()) { innerBlock -> + innerBlock.type in MsgBlock.Type.SIGNED_BLOCK_TYPES + }.forEach { pgpBlock -> + analyzeBlockForPgp(pgpBlock) { hasEncryptedContent, _, _, _, _ -> + if (!isEncrypted) { + isEncrypted = hasEncryptedContent + } + } + } - if (hasSignedContent) { - signedBlockCount++ - } + val displayedBlocks = getDisplayedBlocks(msgBlocks.toList()) + displayedBlocks.filter { innerBlock -> + innerBlock.type in MsgBlock.Type.SIGNED_BLOCK_TYPES + }.forEach { pgpBlock -> + analyzeBlockForPgp(pgpBlock) { _, + hasSignedContent, + hasInvalidSignatures, + keyIdsOfSigningKeys, + verifiedSignaturesList -> + if (hasSignedContent) { + signedBlockCount++ + } - if (!hasBadSignatures) { - hasBadSignatures = hasInvalidSignatures - } + if (!hasBadSignatures) { + hasBadSignatures = hasInvalidSignatures + } - keyIdOfSigningKeys.addAll(keyIdsOfSigningKeys) + keyIdOfSigningKeys.addAll(keyIdsOfSigningKeys) - if (verifiedSignatures.isEmpty()) { + if (verifiedSignatures.isEmpty()) { + verifiedSignatures.addAll(verifiedSignaturesList) + } else { + val keyIdsOfAllVerifiedSignatures = verifiedSignatures.map { it.signingKey.keyId } + val keyIdsOfCurrentVerifiedSignatures = verifiedSignaturesList.map { + it.signingKey.keyId + } + if (keyIdsOfAllVerifiedSignatures != keyIdsOfCurrentVerifiedSignatures) { + hasMixedSignatures = true verifiedSignatures.addAll(verifiedSignaturesList) - } else { - val keyIdsOfAllVerifiedSignatures = verifiedSignatures.map { it.signingKey.keyId } - val keyIdsOfCurrentVerifiedSignatures = verifiedSignaturesList.map { - it.signingKey.keyId - } - if (keyIdsOfAllVerifiedSignatures != keyIdsOfCurrentVerifiedSignatures) { - hasMixedSignatures = true - verifiedSignatures.addAll(verifiedSignaturesList) - } } } } + } + + for (block in msgBlocks) { + // We don't need Base64 correction here, fromAttachment() does this for us + // We also seem to don't need to make correction between raw and utf8 + // But I'd prefer MsgBlock.content to be ByteArray + // So, at least meanwhile, not porting this: + // block.content = isContentBlock(block.type) + // ? block.content.toUtfStr() : block.content.toRawBytesStr(); when { block is DecryptedAndOrSignedContentMsgBlock -> { @@ -907,7 +914,7 @@ object PgpMsg { resultBlocks.add(0, fmtRes.contentBlock) if (signedBlockCount > 0 && - signedBlockCount != msgBlocks.filter { it.type != MsgBlock.Type.ENCRYPTED_SUBJECT }.size + signedBlockCount != displayedBlocks.count { it.type != MsgBlock.Type.ENCRYPTED_SUBJECT } ) { isPartialSigned = true } @@ -1282,32 +1289,20 @@ object PgpMsg { for (block in allContentBlocks.filterNot { MimeUtils.isPlainImgAtt(it) }) { when (block) { is AlternativeContentMsgBlock -> { - if (block.plainBlocks.size > 1) { + val alternativeContentSelection = selectAlternativeContent(block) + if (alternativeContentSelection.usePlainVersionForRendering) { prepareFormattedContentBlock( - allContentBlocks = block.plainBlocks, + allContentBlocks = alternativeContentSelection.displayedBlocks, stripHtmlRootTags = true ).apply { msgContentAsHtml.append(contentBlock.content) msgContentAsText.append(text).append('\n') } - //we skip otherBlocks if we have more than one plain block + //we skip otherBlocks if the plain version was selected for rendering continue } else { - val singlePlainBlock = block.plainBlocks.first() - val singlePlainVersionHasDecryptedContent = - singlePlainBlock is DecryptedAndOrSignedContentMsgBlock - if (singlePlainVersionHasDecryptedContent) { - prepareFormattedContentBlock( - allContentBlocks = singlePlainBlock.blocks, - stripHtmlRootTags = true - ).apply { - msgContentAsHtml.append(contentBlock.content) - msgContentAsText.append(text).append('\n') - } - //we skip otherBlocks if plain version has decrypted content - continue - } else { + block.plainBlocks.firstOrNull()?.let { singlePlainBlock -> collectDataFromMsgBlock( block = singlePlainBlock, useHtml = false, @@ -1316,7 +1311,7 @@ object PgpMsg { } } - val htmlVersionBlock = block.otherBlocks.firstOrNull() + val htmlVersionBlock = alternativeContentSelection.displayedBlocks.firstOrNull() htmlVersionBlock?.let { htmlBlock -> collectDataFromMsgBlock( block = htmlBlock, @@ -1413,6 +1408,42 @@ object PgpMsg { } } + private fun getDisplayedBlocks(blocks: List): List = + blocks.flatMap { block -> + if (block is AlternativeContentMsgBlock) { + getDisplayedBlocks(selectAlternativeContent(block).displayedBlocks) + } else { + listOf(block) + } + } + + private fun selectAlternativeContent( + block: AlternativeContentMsgBlock + ): AlternativeContentSelection { + return when { + block.plainBlocks.size > 1 -> AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) + + block.plainBlocks.singleOrNull() is DecryptedAndOrSignedContentMsgBlock -> + AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) + + block.otherBlocks.isNotEmpty() -> AlternativeContentSelection( + displayedBlocks = listOf(block.otherBlocks.first()), + usePlainVersionForRendering = false + ) + + else -> AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) + } + } + private fun handleMsgBlock( block: MsgBlock, inlineImagesByCid: MutableMap, @@ -1751,6 +1782,11 @@ object PgpMsg { val contentBlock: MsgBlock ) + private data class AlternativeContentSelection( + val displayedBlocks: List, + val usePlainVersionForRendering: Boolean + ) + private enum class FrameColor { GREEN, GRAY, diff --git a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt index bf81cd570f..9aeb9d420f 100644 --- a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt +++ b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt @@ -34,6 +34,33 @@ import java.util.Properties * @author Denys Bondarenko */ class ProcessMimeMessageTest { + @Test + fun testSignedPlainTextDoesNotMarkDisplayedUnsignedHtmlAsSigned() { + val processedMimeMessageResult = PgpMsg.processMimeMessage( + MimeMessage( + Session.getInstance(Properties()), + TestUtil.readResourceAsByteArray( + "mime/signed-plaintext-unsigned-html-alternative.eml" + ).inputStream() + ), + verificationPublicKeys = DENBOND_VERIFICATION_PUBLIC_KEYS, + secretKeys = PGPSecretKeyRingCollection(emptyList()), + protector = SecretKeyRingProtector.unprotectedKeys() + ) + + val verificationResult = processedMimeMessageResult.verificationResult + assertFalse(verificationResult.hasEncryptedParts) + assertFalse(verificationResult.hasSignedParts) + assertFalse(verificationResult.hasMixedSignatures) + assertFalse(verificationResult.isPartialSigned) + assertFalse(verificationResult.hasBadSignatures) + assertFalse(verificationResult.hasUnverifiedSignatures) + + val displayedBlock = processedMimeMessageResult.blocks.first() + assertEquals(MsgBlock.Type.PLAIN_HTML, displayedBlock.type) + assertTrue(requireNotNull(displayedBlock.content).contains("ATTACKER-ACCOUNT")) + } + @Test fun testProcessProtonmailPgpMimeEncrypted() { val processedMimeMessageResult = PgpMsg.processMimeMessage( @@ -602,6 +629,12 @@ class ProcessMimeMessageTest { source = TestUtil.readResourceAsByteArray("pgp/keys/default@flowcrypt.test_fisrtKey_pub.asc") ).pgpKeyRingCollection.pgpPublicKeyRingCollection + private val DENBOND_VERIFICATION_PUBLIC_KEYS = PgpKey.parseKeys( + source = TestUtil.readResourceAsByteArray( + "pgp/keys/denbond7@flowcrypt.test_pub_primary.asc" + ) + ).pgpKeyRingCollection.pgpPublicKeyRingCollection + private val SECRET_KEYS = PgpKey.parseKeys( source = TestUtil.readResourceAsByteArray("pgp/keys/default@flowcrypt.test_fisrtKey_prv_default.asc") ).pgpKeyRingCollection.pgpSecretKeyRingCollection diff --git a/FlowCrypt/src/test/resources/mime/signed-plaintext-unsigned-html-alternative.eml b/FlowCrypt/src/test/resources/mime/signed-plaintext-unsigned-html-alternative.eml new file mode 100644 index 0000000000..f44715e53f --- /dev/null +++ b/FlowCrypt/src/test/resources/mime/signed-plaintext-unsigned-html-alternative.eml @@ -0,0 +1,40 @@ +Return-Path: +Delivered-To: default@flowcrypt.test +Message-ID: +Date: Sat, 20 Jun 2026 12:00:00 +0000 +MIME-Version: 1.0 +To: default@flowcrypt.test +From: DenBond7 +Subject: Signed plaintext alternative, unsigned HTML alternative +Content-Type: multipart/alternative; boundary="fc-alt-signed-plain-unsigned-html" + +--fc-alt-signed-plain-unsigned-html +Content-Type: text/plain; charset=UTF-8; format=flowed +Content-Transfer-Encoding: 7bit + +-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA512 + +It's a cleartext signed message +-----BEGIN PGP SIGNATURE----- +Version: PGPainless + +iHUEARYKACcFAmLs1kUJEMMgic1q+NbOFiEEwWQo1gHLOTeJz2HUwyCJzWr41s4A +AG8wAP44Q3Zzr7sDM9V34A3CdDxMjUFSJcOsPZUuUi1kIZzv6wD/cxdthjQpC7uN +Hwvo1OGuQBE7TENCj1cW4P9AdOFxSAQ= +=McWq +-----END PGP SIGNATURE----- + +--fc-alt-signed-plain-unsigned-html +Content-Type: text/html; charset=UTF-8 +Content-Transfer-Encoding: 7bit + + + +

Unsigned HTML rendered by the message view

+

This visible HTML is not covered by the PGP signature in the text/plain alternative.

+

Example impact text: please approve the new payment destination ATTACKER-ACCOUNT.

+ + + +--fc-alt-signed-plain-unsigned-html-- diff --git a/FlowCrypt/src/test/resources/pgp/keys/denbond7@flowcrypt.test_pub_primary.asc b/FlowCrypt/src/test/resources/pgp/keys/denbond7@flowcrypt.test_pub_primary.asc new file mode 100644 index 0000000000..4683ad85ca --- /dev/null +++ b/FlowCrypt/src/test/resources/pgp/keys/denbond7@flowcrypt.test_pub_primary.asc @@ -0,0 +1,13 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- +Version: PGPainless + +mDMEYIucWBYJKwYBBAHaRw8BAQdAew+8mzMWyf3+Pfy49qa60uKV6e5os7de4TdZ +ceAWUq+0F2RlbmJvbmQ3QGZsb3djcnlwdC50ZXN0iHgEExYKACAFAmCLnFgCGwMF +FgIDAQAECwkIBwUVCgkICwIeAQIZAQAKCRDDIInNavjWzm3JAQCgFgCEyD58iEa/ +Rw/DYNoQNoZC1lhw1bxBiOcIbtkdBgEAsDFZu3TBavOMKI7KW+vfMBHtRVbkMNpv +unaAldoabgO4OARgi5xYEgorBgEEAZdVAQUBAQdAB1/Mrq5JGYim4KqGTSK4OESQ +UwPgK56q0yrkiU9WgyYDAQgHiHUEGBYKAB0FAmCLnFgCGwwFFgIDAQAECwkIBwUV +CgkICwIeAQAKCRDDIInNavjWzjMgAQCU+R1fItqdY6lt9jXUqipmXuqVaEFPwNA8 +YJ1rIwDwVQEAyUc8162KWzA2iQB5akwLwNr/pLDDtOWwhLUkrBb3mAc= +=pXF6 +-----END PGP PUBLIC KEY BLOCK----- From 335056eb9993c9c271ca66b6508ac0600c956a8b Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Fri, 31 Jul 2026 14:30:47 +0300 Subject: [PATCH 2/7] fix(security): prioritize signed plaintext alternative over unsigned HTML - Ensure selectAlternativeContent renders signed text/plain alternative when text/html is unsigned. - Prevent displaying attacker-controlled unsigned HTML under a valid Signed PGP badge. - Update unit and UI regression tests for signed-plaintext-unsigned-html-alternative fixtures. --- .../email/ui/MessageDetailsFlowTest.kt | 6 +++--- .../com/flowcrypt/email/security/pgp/PgpMsg.kt | 17 +++++++++++++++++ .../security/pgp/ProcessMimeMessageTest.kt | 8 ++++++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt b/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt index c15c93b7f8..779fbfcdd3 100644 --- a/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt +++ b/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt @@ -1016,8 +1016,8 @@ class MessageDetailsFlowTest : BaseMessageDetailsFlowTest() { "/html/body", withTextContent( allOf( - containsString("ATTACKER-ACCOUNT"), - not(containsString("It's a cleartext signed message")) + containsString("It's a cleartext signed message"), + not(containsString("ATTACKER-ACCOUNT")) ) ) ) @@ -1027,7 +1027,7 @@ class MessageDetailsFlowTest : BaseMessageDetailsFlowTest() { testPgpBadges( 2, PgpBadgeListAdapter.PgpBadge.Type.NOT_ENCRYPTED, - PgpBadgeListAdapter.PgpBadge.Type.NOT_SIGNED + PgpBadgeListAdapter.PgpBadge.Type.SIGNED ) } diff --git a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt index 993413d438..2b92171967 100644 --- a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt +++ b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt @@ -1420,7 +1420,24 @@ object PgpMsg { private fun selectAlternativeContent( block: AlternativeContentMsgBlock ): AlternativeContentSelection { + val hasSignedPlainBlocks = block.plainBlocks.any { plainBlock -> + filterBlocksViaTree(listOf(plainBlock)) { + it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned + }.isNotEmpty() + } + + val hasSignedOtherBlocks = block.otherBlocks.any { otherBlock -> + filterBlocksViaTree(listOf(otherBlock)) { + it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned + }.isNotEmpty() + } + return when { + hasSignedPlainBlocks && !hasSignedOtherBlocks -> AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) + block.plainBlocks.size > 1 -> AlternativeContentSelection( displayedBlocks = block.plainBlocks, usePlainVersionForRendering = true diff --git a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt index 9aeb9d420f..9330fd0016 100644 --- a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt +++ b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt @@ -50,15 +50,19 @@ class ProcessMimeMessageTest { val verificationResult = processedMimeMessageResult.verificationResult assertFalse(verificationResult.hasEncryptedParts) - assertFalse(verificationResult.hasSignedParts) + assertTrue(verificationResult.hasSignedParts) assertFalse(verificationResult.hasMixedSignatures) assertFalse(verificationResult.isPartialSigned) assertFalse(verificationResult.hasBadSignatures) assertFalse(verificationResult.hasUnverifiedSignatures) + assertTrue(processedMimeMessageResult.text.contains("It's a cleartext signed message")) + assertFalse(processedMimeMessageResult.text.contains("ATTACKER-ACCOUNT")) + val displayedBlock = processedMimeMessageResult.blocks.first() assertEquals(MsgBlock.Type.PLAIN_HTML, displayedBlock.type) - assertTrue(requireNotNull(displayedBlock.content).contains("ATTACKER-ACCOUNT")) + assertTrue(requireNotNull(displayedBlock.content).contains("It's a cleartext signed message")) + assertFalse(requireNotNull(displayedBlock.content).contains("ATTACKER-ACCOUNT")) } @Test From e17b17091cb42a08f097ea734b980d835ec50d6c Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Fri, 31 Jul 2026 14:44:23 +0300 Subject: [PATCH 3/7] `fix(security): keep signed plaintext when displayed HTML is unsigned` --- .../email/ui/MessageDetailsFlowTest.kt | 2 +- .../flowcrypt/email/security/pgp/PgpMsg.kt | 6 +-- .../security/pgp/ProcessMimeMessageTest.kt | 41 ++++++++++++++++++- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt b/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt index 779fbfcdd3..020a4c934b 100644 --- a/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt +++ b/FlowCrypt/src/androidTest/java/com/flowcrypt/email/ui/MessageDetailsFlowTest.kt @@ -998,7 +998,7 @@ class MessageDetailsFlowTest : BaseMessageDetailsFlowTest() { } @Test - fun testSignedPlainTextDoesNotMarkDisplayedUnsignedHtmlAsSigned() { + fun testPrioritizesSignedPlainTextOverUnsignedHtmlAlternative() { PrivateKeysManager.savePubKeyToDatabase("pgp/denbond7@flowcrypt.test_pub_primary.asc") val msgInfo = getMsgInfo( diff --git a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt index 2b92171967..81721cf845 100644 --- a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt +++ b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt @@ -1426,14 +1426,14 @@ object PgpMsg { }.isNotEmpty() } - val hasSignedOtherBlocks = block.otherBlocks.any { otherBlock -> + val hasSignedDisplayedOtherBlock = block.otherBlocks.firstOrNull()?.let { otherBlock -> filterBlocksViaTree(listOf(otherBlock)) { it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned }.isNotEmpty() - } + } == true return when { - hasSignedPlainBlocks && !hasSignedOtherBlocks -> AlternativeContentSelection( + hasSignedPlainBlocks && !hasSignedDisplayedOtherBlock -> AlternativeContentSelection( displayedBlocks = block.plainBlocks, usePlainVersionForRendering = true ) diff --git a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt index 9330fd0016..dad392a396 100644 --- a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt +++ b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt @@ -10,6 +10,7 @@ import com.flowcrypt.email.api.retrofit.response.model.MsgBlock import com.flowcrypt.email.core.msg.MimeUtils import com.flowcrypt.email.extensions.kotlin.toEscapedHtml import com.flowcrypt.email.extensions.kotlin.toInputStream +import com.flowcrypt.email.extensions.kotlin.unescapeHtml import com.flowcrypt.email.util.TestUtil import jakarta.mail.Session import jakarta.mail.internet.MimeMessage @@ -35,7 +36,7 @@ import java.util.Properties */ class ProcessMimeMessageTest { @Test - fun testSignedPlainTextDoesNotMarkDisplayedUnsignedHtmlAsSigned() { + fun testPrioritizesSignedPlainTextOverUnsignedHtmlAlternative() { val processedMimeMessageResult = PgpMsg.processMimeMessage( MimeMessage( Session.getInstance(Properties()), @@ -56,7 +57,9 @@ class ProcessMimeMessageTest { assertFalse(verificationResult.hasBadSignatures) assertFalse(verificationResult.hasUnverifiedSignatures) - assertTrue(processedMimeMessageResult.text.contains("It's a cleartext signed message")) + assertTrue( + processedMimeMessageResult.text.unescapeHtml().contains("It's a cleartext signed message") + ) assertFalse(processedMimeMessageResult.text.contains("ATTACKER-ACCOUNT")) val displayedBlock = processedMimeMessageResult.blocks.first() @@ -65,6 +68,40 @@ class ProcessMimeMessageTest { assertFalse(requireNotNull(displayedBlock.content).contains("ATTACKER-ACCOUNT")) } + @Test + fun testPrioritizesSignedPlainTextWhenLaterAlternativeIsSigned() { + val source = TestUtil.readResourceAsString( + "mime/signed-plaintext-unsigned-html-alternative.eml" + ) + val boundary = "fc-alt-signed-plain-unsigned-html" + val clearSignedContent = source + .substringAfter("Content-Transfer-Encoding: 7bit\n\n") + .substringBefore("\n\n--$boundary") + val sourceWithSignedThirdAlternative = source.replace( + "\n--$boundary--", + "\n--$boundary\n" + + "Content-Type: application/octet-stream\n" + + "Content-Transfer-Encoding: 7bit\n\n" + + clearSignedContent + + "\n\n--$boundary--" + ) + + val processedMimeMessageResult = PgpMsg.processMimeMessage( + MimeMessage( + Session.getInstance(Properties()), + sourceWithSignedThirdAlternative.toInputStream() + ), + verificationPublicKeys = DENBOND_VERIFICATION_PUBLIC_KEYS, + secretKeys = PGPSecretKeyRingCollection(emptyList()), + protector = SecretKeyRingProtector.unprotectedKeys() + ) + + assertTrue(processedMimeMessageResult.verificationResult.hasSignedParts) + val displayedContent = requireNotNull(processedMimeMessageResult.blocks.first().content) + assertTrue(displayedContent.contains("It's a cleartext signed message")) + assertFalse(displayedContent.contains("ATTACKER-ACCOUNT")) + } + @Test fun testProcessProtonmailPgpMimeEncrypted() { val processedMimeMessageResult = PgpMsg.processMimeMessage( From 869f1b45546a985ebefe9dd9b7303a8d6b149af3 Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Mon, 3 Aug 2026 22:24:03 +0300 Subject: [PATCH 4/7] `fix(security): handle signed content in nested MIME alternatives` --- .../flowcrypt/email/security/pgp/PgpMsg.kt | 18 ++--- .../security/pgp/ProcessMimeMessageTest.kt | 66 ++++++++++++++++++- 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt index 81721cf845..657b693737 100644 --- a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt +++ b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt @@ -1417,20 +1417,16 @@ object PgpMsg { } } + private fun hasSignedDisplayedContent(blocks: List): Boolean = + getDisplayedBlocks(blocks).any { + it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned + } + private fun selectAlternativeContent( block: AlternativeContentMsgBlock ): AlternativeContentSelection { - val hasSignedPlainBlocks = block.plainBlocks.any { plainBlock -> - filterBlocksViaTree(listOf(plainBlock)) { - it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned - }.isNotEmpty() - } - - val hasSignedDisplayedOtherBlock = block.otherBlocks.firstOrNull()?.let { otherBlock -> - filterBlocksViaTree(listOf(otherBlock)) { - it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned - }.isNotEmpty() - } == true + val hasSignedPlainBlocks = hasSignedDisplayedContent(block.plainBlocks) + val hasSignedDisplayedOtherBlock = hasSignedDisplayedContent(block.otherBlocks.take(1)) return when { hasSignedPlainBlocks && !hasSignedDisplayedOtherBlock -> AlternativeContentSelection( diff --git a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt index dad392a396..97be9fa6bb 100644 --- a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt +++ b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt @@ -96,9 +96,73 @@ class ProcessMimeMessageTest { protector = SecretKeyRingProtector.unprotectedKeys() ) - assertTrue(processedMimeMessageResult.verificationResult.hasSignedParts) + val verificationResult = processedMimeMessageResult.verificationResult + assertTrue(verificationResult.hasSignedParts) + assertFalse(verificationResult.isPartialSigned) + val displayedContent = requireNotNull(processedMimeMessageResult.blocks.first().content) + assertTrue(displayedContent.contains("It's a cleartext signed message")) + assertFalse(displayedContent.contains("ATTACKER-ACCOUNT")) + } + + @Test + fun testPrioritizesSignedPlainTextOverUnsignedNestedAlternative() { + val source = TestUtil.readResourceAsString( + "mime/signed-plaintext-unsigned-html-alternative.eml" + ) + val boundary = "fc-alt-signed-plain-unsigned-html" + val nestedBoundary = "fc-nested-alt" + val clearSignedContent = source + .substringAfter("Content-Transfer-Encoding: 7bit\n\n") + .substringBefore("\n\n--$boundary") + val alternativeBoundaryMarker = "\n--$boundary\n" + val firstAlternativeBoundaryIndex = source.indexOf(alternativeBoundaryMarker) + val secondAlternativeBoundaryIndex = source.indexOf( + alternativeBoundaryMarker, + firstAlternativeBoundaryIndex + alternativeBoundaryMarker.length + ) + val closingBoundaryIndex = source.lastIndexOf("\n--$boundary--") + val nestedAlternative = listOf( + "Content-Type: multipart/alternative; boundary=\"$nestedBoundary\"", + "", + "--$nestedBoundary", + "Content-Type: text/plain; charset=UTF-8", + "Content-Transfer-Encoding: 7bit", + "", + "Unsigned nested plaintext", + "--$nestedBoundary", + "Content-Type: text/html; charset=UTF-8", + "Content-Transfer-Encoding: 7bit", + "", + "Nested unsigned HTML ATTACKER-ACCOUNT", + "--$nestedBoundary", + "Content-Type: application/octet-stream", + "Content-Transfer-Encoding: 7bit", + "", + clearSignedContent, + "--$nestedBoundary--" + ).joinToString("\n") + val sourceWithNestedAlternative = source.replaceRange( + secondAlternativeBoundaryIndex + alternativeBoundaryMarker.length, + closingBoundaryIndex, + nestedAlternative + ) + + val processedMimeMessageResult = PgpMsg.processMimeMessage( + MimeMessage( + Session.getInstance(Properties()), + sourceWithNestedAlternative.toInputStream() + ), + verificationPublicKeys = DENBOND_VERIFICATION_PUBLIC_KEYS, + secretKeys = PGPSecretKeyRingCollection(emptyList()), + protector = SecretKeyRingProtector.unprotectedKeys() + ) + + val verificationResult = processedMimeMessageResult.verificationResult + assertTrue(verificationResult.hasSignedParts) + assertFalse(verificationResult.isPartialSigned) val displayedContent = requireNotNull(processedMimeMessageResult.blocks.first().content) assertTrue(displayedContent.contains("It's a cleartext signed message")) + assertFalse(displayedContent.contains("Unsigned nested plaintext")) assertFalse(displayedContent.contains("ATTACKER-ACCOUNT")) } From 694f54855a233d93f269fbec2808ca6c012d0bd5 Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Wed, 5 Aug 2026 22:08:51 +0300 Subject: [PATCH 5/7] fix(issue_3231_fix_unsigned-HTML-alternative): cache nested MIME alternative selection --- .../flowcrypt/email/security/pgp/PgpMsg.kt | 94 +++++++++++-------- .../security/pgp/ProcessMimeMessageTest.kt | 45 +++++++++ 2 files changed, 101 insertions(+), 38 deletions(-) diff --git a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt index 657b693737..f6b994d32d 100644 --- a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt +++ b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt @@ -70,6 +70,7 @@ import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.InputStream import java.nio.charset.StandardCharsets +import java.util.IdentityHashMap import java.util.Properties import kotlin.random.Random @@ -835,6 +836,7 @@ object PgpMsg { var isPartialSigned = false val verifiedSignatures = mutableListOf() val keyIdOfSigningKeys = mutableSetOf() + val alternativeContentResolver = AlternativeContentResolver() filterBlocksViaTree(msgBlocks.toList()) { innerBlock -> innerBlock.type in MsgBlock.Type.SIGNED_BLOCK_TYPES @@ -846,7 +848,7 @@ object PgpMsg { } } - val displayedBlocks = getDisplayedBlocks(msgBlocks.toList()) + val displayedBlocks = alternativeContentResolver.getDisplayedBlocks(msgBlocks.toList()) displayedBlocks.filter { innerBlock -> innerBlock.type in MsgBlock.Type.SIGNED_BLOCK_TYPES }.forEach { pgpBlock -> @@ -910,7 +912,10 @@ object PgpMsg { } } - val fmtRes = prepareFormattedContentBlock(contentBlocks) + val fmtRes = prepareFormattedContentBlock( + allContentBlocks = contentBlocks, + alternativeContentResolver = alternativeContentResolver + ) resultBlocks.add(0, fmtRes.contentBlock) if (signedBlockCount > 0 && @@ -1250,6 +1255,7 @@ object PgpMsg { private fun prepareFormattedContentBlock( allContentBlocks: List, + alternativeContentResolver: AlternativeContentResolver, stripHtmlRootTags: Boolean = false ): FormattedContentBlockResult { val inlineImagesByCid = mutableMapOf() @@ -1289,10 +1295,11 @@ object PgpMsg { for (block in allContentBlocks.filterNot { MimeUtils.isPlainImgAtt(it) }) { when (block) { is AlternativeContentMsgBlock -> { - val alternativeContentSelection = selectAlternativeContent(block) + val alternativeContentSelection = alternativeContentResolver.select(block) if (alternativeContentSelection.usePlainVersionForRendering) { prepareFormattedContentBlock( allContentBlocks = alternativeContentSelection.displayedBlocks, + alternativeContentResolver = alternativeContentResolver, stripHtmlRootTags = true ).apply { msgContentAsHtml.append(contentBlock.content) @@ -1324,6 +1331,7 @@ object PgpMsg { is DecryptedAndOrSignedContentMsgBlock -> { prepareFormattedContentBlock( allContentBlocks = block.blocks, + alternativeContentResolver = alternativeContentResolver, stripHtmlRootTags = true ).apply { msgContentAsHtml.append(contentBlock.content) @@ -1408,52 +1416,62 @@ object PgpMsg { } } - private fun getDisplayedBlocks(blocks: List): List = - blocks.flatMap { block -> - if (block is AlternativeContentMsgBlock) { - getDisplayedBlocks(selectAlternativeContent(block).displayedBlocks) - } else { - listOf(block) + private class AlternativeContentResolver { + private val selectionCache = + IdentityHashMap() + + fun getDisplayedBlocks(blocks: List): List = + blocks.flatMap { block -> + if (block is AlternativeContentMsgBlock) { + getDisplayedBlocks(select(block).displayedBlocks) + } else { + listOf(block) + } } - } - private fun hasSignedDisplayedContent(blocks: List): Boolean = - getDisplayedBlocks(blocks).any { - it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned - } + fun select(block: AlternativeContentMsgBlock): AlternativeContentSelection = + selectionCache.getOrPut(block) { + selectNotCached(block) + } - private fun selectAlternativeContent( - block: AlternativeContentMsgBlock - ): AlternativeContentSelection { - val hasSignedPlainBlocks = hasSignedDisplayedContent(block.plainBlocks) - val hasSignedDisplayedOtherBlock = hasSignedDisplayedContent(block.otherBlocks.take(1)) + private fun hasSignedDisplayedContent(blocks: List): Boolean = + getDisplayedBlocks(blocks).any { + it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned + } - return when { - hasSignedPlainBlocks && !hasSignedDisplayedOtherBlock -> AlternativeContentSelection( - displayedBlocks = block.plainBlocks, - usePlainVersionForRendering = true - ) + private fun selectNotCached( + block: AlternativeContentMsgBlock + ): AlternativeContentSelection { + val hasSignedPlainBlocks = hasSignedDisplayedContent(block.plainBlocks) + val hasSignedDisplayedOtherBlock = hasSignedDisplayedContent(block.otherBlocks.take(1)) - block.plainBlocks.size > 1 -> AlternativeContentSelection( - displayedBlocks = block.plainBlocks, - usePlainVersionForRendering = true - ) + return when { + hasSignedPlainBlocks && !hasSignedDisplayedOtherBlock -> AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) - block.plainBlocks.singleOrNull() is DecryptedAndOrSignedContentMsgBlock -> - AlternativeContentSelection( + block.plainBlocks.size > 1 -> AlternativeContentSelection( displayedBlocks = block.plainBlocks, usePlainVersionForRendering = true ) - block.otherBlocks.isNotEmpty() -> AlternativeContentSelection( - displayedBlocks = listOf(block.otherBlocks.first()), - usePlainVersionForRendering = false - ) + block.plainBlocks.singleOrNull() is DecryptedAndOrSignedContentMsgBlock -> + AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) - else -> AlternativeContentSelection( - displayedBlocks = block.plainBlocks, - usePlainVersionForRendering = true - ) + block.otherBlocks.isNotEmpty() -> AlternativeContentSelection( + displayedBlocks = listOf(block.otherBlocks.first()), + usePlainVersionForRendering = false + ) + + else -> AlternativeContentSelection( + displayedBlocks = block.plainBlocks, + usePlainVersionForRendering = true + ) + } } } diff --git a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt index 97be9fa6bb..a5f6924e58 100644 --- a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt +++ b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/ProcessMimeMessageTest.kt @@ -166,6 +166,51 @@ class ProcessMimeMessageTest { assertFalse(displayedContent.contains("ATTACKER-ACCOUNT")) } + @Test(timeout = 5_000) + fun testProcessesDeeplyNestedAlternativesWithoutRepeatedSelection() { + val nestedContent = (0 until 25).fold( + listOf( + "Content-Type: text/html; charset=UTF-8", + "Content-Transfer-Encoding: 7bit", + "", + "Deeply nested unsigned HTML" + ).joinToString("\n") + ) { innerContent, depth -> + val boundary = "fc-deep-alt-$depth" + listOf( + "Content-Type: multipart/alternative; boundary=\"$boundary\"", + "", + "--$boundary", + "Content-Type: text/plain; charset=UTF-8", + "Content-Transfer-Encoding: 7bit", + "", + "Unsigned plaintext at depth $depth", + "--$boundary", + innerContent, + "--$boundary--" + ).joinToString("\n") + } + val source = listOf( + "From: sender@flowcrypt.test", + "To: default@flowcrypt.test", + "Subject: Deeply nested alternatives", + "MIME-Version: 1.0", + nestedContent + ).joinToString("\n") + + val processedMimeMessageResult = PgpMsg.processMimeMessage( + MimeMessage( + Session.getInstance(Properties()), + source.toInputStream() + ), + verificationPublicKeys = PGPPublicKeyRingCollection(emptyList()), + secretKeys = PGPSecretKeyRingCollection(emptyList()), + protector = SecretKeyRingProtector.unprotectedKeys() + ) + + assertFalse(processedMimeMessageResult.verificationResult.hasSignedParts) + } + @Test fun testProcessProtonmailPgpMimeEncrypted() { val processedMimeMessageResult = PgpMsg.processMimeMessage( From b5722331f3fc9fcc8a3eb30f7428c9e12681a40a Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Mon, 10 Aug 2026 10:17:19 +0300 Subject: [PATCH 6/7] `fix(issue_3231_fix_unsigned-HTML-alternative): exclude images from rejected MIME alternatives` --- .../flowcrypt/email/security/pgp/PgpMsg.kt | 29 +++- .../pgp/PgpMsgInlineImageAlternativeTest.kt | 132 ++++++++++++++++++ 2 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/PgpMsgInlineImageAlternativeTest.kt diff --git a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt index f6b994d32d..165ced2482 100644 --- a/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt +++ b/FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt @@ -1260,9 +1260,8 @@ object PgpMsg { ): FormattedContentBlockResult { val inlineImagesByCid = mutableMapOf() val imagesAtTheBottom = mutableListOf() - val plainImageBlocks = filterBlocksViaTree(allContentBlocks) { - MimeUtils.isPlainImgAtt(it) - } + val plainImageBlocks = + alternativeContentResolver.getInlineImageBlocksForRendering(allContentBlocks) for (plainImageBlock in plainImageBlocks) { var contentId = (plainImageBlock as AttMsgBlock).attMeta.contentId ?: "" if (contentId.isNotEmpty()) { @@ -1434,6 +1433,30 @@ object PgpMsg { selectNotCached(block) } + fun getInlineImageBlocksForRendering(blocks: List): List = + blocks.flatMap { block -> + when { + block is AlternativeContentMsgBlock -> { + val selection = select(block) + val blocksToInspect = when { + hasSignedDisplayedContent(selection.displayedBlocks) -> { + // Do not mix unsigned images from a rejected alternative into signed content. + selection.displayedBlocks + block.otherBlocks.drop(1).filter { + MimeUtils.isPlainImgAtt(it) && it.isOpenPGPMimeSigned + } + } + + selection.usePlainVersionForRendering -> block.allBlocks + else -> block.otherBlocks + } + getInlineImageBlocksForRendering(blocksToInspect) + } + + MimeUtils.isPlainImgAtt(block) -> listOf(block) + else -> emptyList() + } + } + private fun hasSignedDisplayedContent(blocks: List): Boolean = getDisplayedBlocks(blocks).any { it.type in MsgBlock.Type.SIGNED_BLOCK_TYPES || it.isOpenPGPMimeSigned diff --git a/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/PgpMsgInlineImageAlternativeTest.kt b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/PgpMsgInlineImageAlternativeTest.kt new file mode 100644 index 0000000000..68ea92f4ef --- /dev/null +++ b/FlowCrypt/src/test/java/com/flowcrypt/email/security/pgp/PgpMsgInlineImageAlternativeTest.kt @@ -0,0 +1,132 @@ +/* + * © 2016-present FlowCrypt a.s. Limitations apply. Contact human@flowcrypt.com + * Contributors: denbond7 + */ + +package com.flowcrypt.email.security.pgp + +import com.flowcrypt.email.BuildConfig +import com.flowcrypt.email.extensions.kotlin.toInputStream +import com.flowcrypt.email.util.TestUtil +import jakarta.mail.Session +import jakarta.mail.internet.MimeMessage +import org.bouncycastle.openpgp.PGPPublicKeyRingCollection +import org.bouncycastle.openpgp.PGPSecretKeyRingCollection +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.pgpainless.key.protection.SecretKeyRingProtector +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.util.Properties + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [BuildConfig.MIN_SDK_VERSION]) +class PgpMsgInlineImageAlternativeTest { + @Test + fun testRejectsInlineImageFromUnsignedHtmlAlternative() { + val source = TestUtil.readResourceAsString( + "mime/signed-plaintext-unsigned-html-alternative.eml" + ) + val sourceWithRelatedHtml = addInlineImageToHtmlAlternative(source) + + val processedMimeMessageResult = PgpMsg.processMimeMessage( + MimeMessage( + Session.getInstance(Properties()), + sourceWithRelatedHtml.toInputStream() + ), + verificationPublicKeys = DENBOND_VERIFICATION_PUBLIC_KEYS, + secretKeys = PGPSecretKeyRingCollection(emptyList()), + protector = SecretKeyRingProtector.unprotectedKeys() + ) + + val verificationResult = processedMimeMessageResult.verificationResult + assertTrue(verificationResult.hasSignedParts) + assertFalse(verificationResult.isPartialSigned) + val displayedContent = requireNotNull(processedMimeMessageResult.blocks.first().content) + assertTrue(displayedContent.contains("It's a cleartext signed message")) + assertFalse(displayedContent.contains("ATTACKER-ACCOUNT")) + assertFalse(displayedContent.contains("attacker.png")) + } + + @Test + fun testKeepsInlineImageFromDisplayedUnsignedHtmlAlternative() { + val source = TestUtil.readResourceAsString( + "mime/signed-plaintext-unsigned-html-alternative.eml" + ) + val boundary = "fc-alt-signed-plain-unsigned-html" + val clearSignedContent = source + .substringAfter("Content-Transfer-Encoding: 7bit\n\n") + .substringBefore("\n\n--$boundary") + val sourceWithUnsignedPlainText = source.replace( + clearSignedContent, + "Unsigned plaintext fallback" + ) + val sourceWithRelatedHtml = addInlineImageToHtmlAlternative(sourceWithUnsignedPlainText) + + val processedMimeMessageResult = PgpMsg.processMimeMessage( + MimeMessage( + Session.getInstance(Properties()), + sourceWithRelatedHtml.toInputStream() + ), + verificationPublicKeys = PGPPublicKeyRingCollection(emptyList()), + secretKeys = PGPSecretKeyRingCollection(emptyList()), + protector = SecretKeyRingProtector.unprotectedKeys() + ) + + assertFalse(processedMimeMessageResult.verificationResult.hasSignedParts) + val displayedContent = requireNotNull(processedMimeMessageResult.blocks.first().content) + assertTrue(displayedContent.contains("ATTACKER-ACCOUNT")) + assertTrue(displayedContent.contains("src=\"data:image/png")) + assertTrue(displayedContent.contains("alt=\"attacker.png\"")) + } + + private fun addInlineImageToHtmlAlternative(source: String): String { + val boundary = "fc-alt-signed-plain-unsigned-html" + val relatedBoundary = "fc-related-html-image" + val alternativeBoundaryMarker = "\n--$boundary\n" + val firstAlternativeBoundaryIndex = source.indexOf(alternativeBoundaryMarker) + val secondAlternativeBoundaryIndex = source.indexOf( + alternativeBoundaryMarker, + firstAlternativeBoundaryIndex + alternativeBoundaryMarker.length + ) + val closingBoundaryIndex = source.lastIndexOf("\n--$boundary--") + val htmlAlternative = source.substring( + secondAlternativeBoundaryIndex + alternativeBoundaryMarker.length, + closingBoundaryIndex + ).trimEnd().replace( + "", + "\"attacker.png\"" + ) + val relatedHtmlAlternative = listOf( + "Content-Type: multipart/related; boundary=\"$relatedBoundary\"", + "", + "--$relatedBoundary", + htmlAlternative, + "--$relatedBoundary", + "Content-Type: image/png; name=\"attacker.png\"", + "Content-Disposition: inline; filename=\"attacker.png\"", + "Content-Transfer-Encoding: base64", + "Content-ID: ", + "", + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Wl2n0Y" + + "AAAAASUVORK5CYII=", + "--$relatedBoundary--" + ).joinToString("\n") + + return source.replaceRange( + secondAlternativeBoundaryIndex + alternativeBoundaryMarker.length, + closingBoundaryIndex, + relatedHtmlAlternative + ) + } + + companion object { + private val DENBOND_VERIFICATION_PUBLIC_KEYS = PgpKey.parseKeys( + source = TestUtil.readResourceAsByteArray( + "pgp/keys/denbond7@flowcrypt.test_pub_primary.asc" + ) + ).pgpKeyRingCollection.pgpPublicKeyRingCollection + } +} From eb4ab96fd1c02857fad4f724e3833b84e22bdb06 Mon Sep 17 00:00:00 2001 From: Denys Bondarenko Date: Mon, 10 Aug 2026 11:55:43 +0300 Subject: [PATCH 7/7] `fix(issue_3231_fix_unsigned-HTML-alternative): updated gradle to 9.7.0` --- gradle/wrapper/gradle-wrapper.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index a351597e62..1e922f4074 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-9.6.1-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-9.7.0-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME