Skip to content

Issue 3231 fix unsigned html alternative - #3244

Open
DenBond7 wants to merge 7 commits into
masterfrom
issue_3231_fix_unsigned-HTML-alternative
Open

Issue 3231 fix unsigned html alternative#3244
DenBond7 wants to merge 7 commits into
masterfrom
issue_3231_fix_unsigned-HTML-alternative

Conversation

@DenBond7

@DenBond7 DenBond7 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

This PR resolves a security vulnerability where an unsigned HTML alternative body could be displayed while simultaneously presenting a Signed PGP badge when an email contains a cleartext/inline signed text/plain alternative alongside an unsigned text/html alternative.

🔒 Problem & Context

Previously, when parsing multipart/alternative emails:

  1. Alternative-content selection did not recognize inline cleartext-signed blocks (SignedMsgBlock) in plainBlocks, causing it to fall through and select the first otherBlocks entry (unsigned text/html) for rendering.
  2. The UI presented the attacker-controlled unsigned HTML body while displaying a Signed PGP badge verified against the plaintext alternative, violating the signature-to-content binding boundary.
  3. Evaluating signedness over nested multipart/alternative structures without selection caching resulted in exponential recomputation, allowing deeply nested alternatives to stall message processing.

🛠 What was done

  • Prioritize Signed Alternatives (PgpMsg.kt):
    Updated AlternativeContentResolver to determine whether plainBlocks contains PGP-signed content (SIGNED_BLOCK_TYPES or isOpenPGPMimeSigned). If plainBlocks is signed and the first candidate in otherBlocks is unsigned, the resolver selects the signed plaintext version for rendering.

  • Displayed Candidate Resolution (PgpMsg.kt):
    Added hasSignedDisplayedContent, which first resolves nested alternatives through getDisplayedBlocks() and then checks only the blocks that will actually be displayed. At the current alternative level, only otherBlocks.take(1) is considered because it represents the rendering candidate. This prevents hidden or later signed parts inside nested alternatives from incorrectly influencing the outer alternative selection.

  • Exponential Recursion Mitigation (AlternativeContentResolver):
    Encapsulated alternative resolution in AlternativeContentResolver, backed by an IdentityHashMap selection cache. This ensures each AlternativeContentMsgBlock selection is evaluated at most once and avoids exponential recomputation for deeply nested MIME structures.

  • Selection-Aware Inline Images (PgpMsg.kt):
    Updated inline-image collection to follow the selected MIME alternative. Unsigned inline images from a rejected HTML alternative are no longer included when signed plaintext is selected, while CID images belonging to the displayed HTML alternative remain supported.

  • Regression Tests (ProcessMimeMessageTest.kt, PgpMsgInlineImageAlternativeTest.kt, and MessageDetailsFlowTest.kt):

    • Updated unit tests to verify that signed plaintext alternatives are selected for rendering over unsigned HTML alternatives, including the correct handling of HTML entities.
    • Added a nested multipart/alternative regression test where the nested displayed HTML is unsigned but a later hidden nested part is signed. The test verifies that the valid signed outer plaintext is still selected and that hidden signed content cannot influence the selection.
    • Added a test case covering a multipart/alternative message with three alternatives: signed plaintext, unsigned first HTML, and a signed third alternative. This ensures that only the actual rendering candidate affects selection.
    • Added a performance regression test (testProcessesDeeplyNestedAlternativesWithoutRepeatedSelection) with 25 levels of nested multipart/alternative structures to guard against a recurrence of exponential processing slowdown.
    • Added regression tests verifying that unsigned inline images from a rejected HTML alternative are excluded, while legitimate CID images from the displayed HTML alternative are preserved.
    • Updated UI instrumentation tests to verify that web content displays the verified signed text alongside the Signed PGP badge.

Implementation details

Before

filterBlocksViaTree ran inside:

for (block in msgBlocks)

It analyzed all MIME alternatives when determining both encryption and signature status.

For a message containing:

  • signed text/plain;
  • unsigned text/html;

the function detected the signature in the plaintext alternative, while the renderer selected the HTML alternative.

As a result, unsigned HTML content was displayed with a Signed badge.

After

The processing is now split into three sequential phases:

  1. All MIME blocks are scanned to preserve the existing encryption-status calculation.
  2. Only the blocks that will actually be displayed are resolved and checked for signatures using the cached AlternativeContentResolver.
  3. The remaining loop separates content blocks from result blocks, and formatting uses the same cached alternative selection.
All MIME blocks
│
├── Check encryption across the entire MIME tree
│
├── Select the alternative that will be displayed (cached via AlternativeContentResolver)
│   └── Check signatures only for the displayed blocks
│
└── Build contentBlocks/resultBlocks
    └── Render the same selected alternative

getDisplayedBlocks() resolves the alternative branch selected for rendering. Only the resolved displayed blocks are used to calculate the signature status.

The remaining loop:

for (block in msgBlocks)

now only separates content blocks from result blocks for formatting.

Why

Signature status must describe the content that is actually displayed.

Moving signature analysis outside the original loop separates:

  • whole-message encryption analysis;
  • displayed-content signature analysis.

This prevents a signature from a hidden MIME alternative from being applied to the alternative selected for rendering.

close #3231


Tests (delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities

DenBond7 added 4 commits July 31, 2026 14:06
…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.
@DenBond7
DenBond7 marked this pull request as ready for review July 31, 2026 13:53
@DenBond7
DenBond7 requested a review from sosnovsky as a code owner July 31, 2026 13:53
@DenBond7

Copy link
Copy Markdown
Collaborator Author

@sosnovsky This one is ready

Comment thread FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt Outdated
@DenBond7
DenBond7 requested a review from sosnovsky August 4, 2026 04:55
Comment thread FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt Outdated
@DenBond7
DenBond7 requested a review from sosnovsky August 6, 2026 01:45
Comment thread FlowCrypt/src/main/java/com/flowcrypt/email/security/pgp/PgpMsg.kt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FlowCrypt Android unsigned HTML alternative can be shown under a Signed badge

2 participants