#5011 Fix: Image with CID (Content-ID) not displaying in pgp block (#5024)

* feat: display cid image with base64 data

* get rid of img-to-link

* fix: content type

* fix: ui test

* feat: added ui test

* fix: ui test

Author: Ioan Moldovan

extension/chrome/elements/pgp_block_modules/pgp-block-quote-module.ts

@@ -11,7 +11,7 @@ export class PgpBlockViewQuoteModule {
 
   public separateQuotedContentAndRenderText = async (decryptedContent: string, isHtml: boolean) => {
     if (isHtml) {
-      const message = $('<div>').html(Xss.htmlSanitizeKeepBasicTags(decryptedContent, 'IMG-TO-LINK')); // xss-sanitized
+      const message = $('<div>').html(Xss.htmlSanitizeKeepBasicTags(decryptedContent)); // xss-sanitized
       let htmlBlockQuoteExists = false;
       const shouldBeQuoted: Array<Element> = [];
       for (let i = message[0].children.length - 1; i >= 0; i--) {
@@ -75,7 +75,7 @@ export class PgpBlockViewQuoteModule {
       '<div id="action_show_quoted_content" data-test="action-show-quoted-content" class="three_dots"><img src="/img/svgs/three-dots.svg" /></div>'
     ); // xss-direct
     const messageHtml = isHtml ? message : Str.escapeTextAsRenderableHtml(message);
-    pgpBlk.append(`<div class="quoted_content">${Xss.htmlSanitizeKeepBasicTags(messageHtml, 'IMG-TO-LINK')}</div>`); // xss-sanitized
+    pgpBlk.append(`<div class="quoted_content">${Xss.htmlSanitizeKeepBasicTags(messageHtml)}</div>`); // xss-sanitized
     pgpBlk.find('#action_show_quoted_content').on(
       'click',
       this.view.setHandler(() => {

extension/chrome/elements/pgp_block_modules/pgp-block-render-module.ts

@@ -14,7 +14,8 @@ import { Xss } from '../../../js/common/platform/xss.js';
 import { MsgBlockParser } from '../../../js/common/core/msg-block-parser.js';
 import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
 import { GmailParser } from '../../../js/common/api/email-provider/gmail/gmail-parser.js';
-import { Str } from '../../../js/common/core/common.js';
+import { CID_PATTERN, Str } from '../../../js/common/core/common.js';
+import DOMPurify from 'dompurify';
 
 export class PgpBlockViewRenderModule {
   public doNotSetStateAsReadyYet = false;
@@ -179,16 +180,12 @@ export class PgpBlockViewRenderModule {
     if (!isErr) {
       // rendering message content
       $('.pgp_print_button').show();
-      const pgpBlock = $('#pgp_block').html(Xss.htmlSanitizeKeepBasicTags(htmlContent, 'IMG-TO-LINK')); // xss-sanitized
+      $('#pgp_block').html(Xss.htmlSanitizeKeepBasicTags(htmlContent)); // xss-sanitized
       Xss.appendRemoteImagesToContainer();
       $('#pgp_block .remote_image_container img').on(
         'load',
         this.view.setHandler(() => this.resizePgpBlockFrame())
       );
-      pgpBlock.find('a.image_src_link').one(
-        'click',
-        this.view.setHandler((el, ev) => this.displayImageSrcLinkAsImg(el as HTMLAnchorElement, ev as JQuery.Event<HTMLAnchorElement, null>))
-      );
     } else {
       // rendering our own ui
       Xss.sanitizeRender('#pgp_block', htmlContent);
@@ -280,8 +277,9 @@ export class PgpBlockViewRenderModule {
     } else {
       this.renderText('Formatting...');
       const decoded = await Mime.decode(decryptedBytes);
+      let inlineCIDAttachments: Attachment[] = [];
       if (typeof decoded.html !== 'undefined') {
-        decryptedContent = decoded.html;
+        ({ sanitizedHtml: decryptedContent, inlineCIDAttachments } = this.replaceInlineImageCIDs(decoded.html, decoded.attachments));
         isHtml = true;
       } else if (typeof decoded.text !== 'undefined') {
         decryptedContent = decoded.text;
@@ -299,7 +297,7 @@ export class PgpBlockViewRenderModule {
       for (const attachment of decoded.attachments) {
         if (attachment.isPublicKey()) {
           publicKeys.push(attachment.getData().toUtfStr());
-        } else {
+        } else if (!inlineCIDAttachments.some(inlineAttachment => inlineAttachment.cid === attachment.cid)) {
           renderableAttachments.push(attachment);
         }
       }
@@ -319,36 +317,46 @@ export class PgpBlockViewRenderModule {
     }
   };
 
-  private displayImageSrcLinkAsImg = (a: HTMLAnchorElement, event: JQuery.Event<HTMLAnchorElement, null>) => {
-    const img = document.createElement('img');
-    img.setAttribute('style', a.getAttribute('style') || '');
-    img.style.background = 'none';
-    img.style.border = 'none';
-    img.addEventListener('load', () => this.resizePgpBlockFrame());
-    if (a.href.startsWith('cid:')) {
-      // image included in the email
-      const contentId = a.href.replace(/^cid:/g, '');
-      const content = this.view.attachmentsModule.includedAttachments.filter(a => a.type.indexOf('image/') === 0 && a.cid === `<${contentId}>`)[0];
-      if (content) {
-        img.src = `data:${a.type};base64,${content.getData().toBase64Str()}`;
-        Xss.replaceElementDANGEROUSLY(a, img.outerHTML); // xss-safe-value - img.outerHTML was built using dom node api
-      } else {
-        Xss.replaceElementDANGEROUSLY(a, Xss.escape(`[broken link: ${a.href}]`)); // xss-escaped
+  /**
+   * Replaces inline image CID references with base64 encoded data in sanitized HTML
+   * and returns the sanitized HTML along with the inline CID attachments.
+   *
+   * @param html - The original HTML content.
+   * @param attachments - An array of email attachments.
+   * @returns An object containing sanitized HTML and an array of inline CID attachments.
+   */
+  private replaceInlineImageCIDs = (html: string, attachments: Attachment[]): { sanitizedHtml: string; inlineCIDAttachments: Attachment[] } => {
+    // Array to store inline CID attachments
+    const inlineCIDAttachments: Attachment[] = [];
+
+    // Define the hook function for DOMPurify to process image elements after sanitizing attributes
+    const processImageElements = (node: Element | null) => {
+      // Ensure the node exists and has a 'src' attribute
+      if (!node || !('src' in node)) return;
+      const imageSrc = node.getAttribute('src') as string;
+      const matches = imageSrc.match(CID_PATTERN);
+
+      // Check if the src attribute contains a CID
+      if (matches && matches[1]) {
+        const contentId = matches[1];
+        const contentIdAttachment = attachments.find(attachment => attachment.cid === `<${contentId}>`);
+
+        // Replace the src attribute with a base64 encoded string
+        if (contentIdAttachment) {
+          inlineCIDAttachments.push(contentIdAttachment);
+          node.setAttribute('src', `data:${contentIdAttachment.type};base64,${contentIdAttachment.getData().toBase64Str()}`);
+        }
       }
-    } else if (a.href.startsWith('https://') || a.href.startsWith('http://')) {
-      // image referenced as url
-      img.src = a.href;
-      Xss.replaceElementDANGEROUSLY(a, img.outerHTML); // xss-safe-value - img.outerHTML was built using dom node api
-    } else if (a.href.startsWith('data:image/')) {
-      // image directly inlined
-      img.src = a.href;
-      Xss.replaceElementDANGEROUSLY(a, img.outerHTML); // xss-safe-value - img.outerHTML was built using dom node api
-    } else {
-      Xss.replaceElementDANGEROUSLY(a, Xss.escape(`[broken link: ${a.href}]`)); // xss-escaped
-    }
-    event.preventDefault();
-    event.stopPropagation();
-    event.stopImmediatePropagation();
+    };
+
+    // Add the DOMPurify hook
+    DOMPurify.addHook('afterSanitizeAttributes', processImageElements);
+
+    // Sanitize the HTML and remove the DOMPurify hooks
+    const sanitizedHtml = DOMPurify.sanitize(html);
+    DOMPurify.removeAllHooks();
+
+    return { sanitizedHtml, inlineCIDAttachments };
   };
 
   private getEncryptedSubjectText = (subject: string, isHtml: boolean) => {

extension/js/common/core/common.ts

@@ -12,6 +12,8 @@ export type UrlParams = Dict<UrlParam>;
 export type PromiseCancellation = { cancel: boolean };
 export type EmailParts = { email: string; name?: string };
 
+export const CID_PATTERN = /^cid:(.+)/;
+
 export class Str {
   // ranges are taken from https://stackoverflow.com/a/14824756
   // with the '\u0300' -> '\u0370' modification, because from '\u0300' to '\u0370' there are only punctuation marks
@@ -416,3 +418,8 @@ export const asyncSome = async <T>(arr: Array<T>, predicate: (e: T) => Promise<b
 export const stringTuple = <T extends string[]>(...data: T): T => {
   return data;
 };
+
+export const checkValidURL = (url: string): boolean => {
+  const pattern = /(http|https):\/\/([a-z0-9-]+((\.[a-z0-9-]+)+)?)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@\-\/]))?/;
+  return pattern.test(url);
+};

extension/js/common/core/msg-block-parser.ts

@@ -46,7 +46,7 @@ export class MsgBlockParser {
 
   public static fmtDecryptedAsSanitizedHtmlBlocks = async (
     decryptedContent: Uint8Array,
-    imgHandling: SanitizeImgHandling = 'IMG-TO-LINK'
+    imgHandling: SanitizeImgHandling = 'IMG-KEEP'
   ): Promise<SanitizedBlocks> => {
     const blocks: MsgBlock[] = [];
     let isRichText = false;

extension/js/common/platform/xss.ts

@@ -4,9 +4,9 @@
 
 import * as DOMPurify from 'dompurify';
 
-import { Str } from '../core/common.js';
+import { checkValidURL, CID_PATTERN, Str } from '../core/common.js';
 
-export type SanitizeImgHandling = 'IMG-DEL' | 'IMG-KEEP' | 'IMG-TO-LINK' | 'IMG-TO-PLAIN-URL';
+export type SanitizeImgHandling = 'IMG-DEL' | 'IMG-KEEP' | 'IMG-TO-PLAIN-TEXT';
 
 /**
  * This class is in platform/ folder because most of it depends on platform specific code
@@ -98,13 +98,8 @@ export class Xss {
    * @property {string} imgHandling   - how should images be treated, with the following options:
    *   - IMG-DEL: remove images, only leaving text
    *   - IMG-KEEP: keep images as they are
-   *   - IMG-TO-LINK: transform images to clickable links that display the images inline upon click, as follows:
-   *          from: <img src="there" title="that">
-   *          to:   <a href="data:image/..." title="that" class="image_src_link" target="_blank" style="...">show image</a>
-   *          or:   <a href="https://..." title="that" class="image_src_link" target="_blank" style="...">show image (remote)</a>
-   *          (when rendered, we add event handler to `.image_src_link` that responds to a click and render the image)
    */
-  public static htmlSanitizeKeepBasicTags = (dirtyHtml: string, imgHandling: SanitizeImgHandling): string => {
+  public static htmlSanitizeKeepBasicTags = (dirtyHtml: string, imgHandling: SanitizeImgHandling = 'IMG-KEEP'): string => {
     Xss.throwIfNotSupported();
     // used whenever untrusted remote content (eg html email) is rendered, but we still want to preserve html
     DOMPurify.removeAllHooks();
@@ -131,33 +126,14 @@ export class Xss {
           img.remove(); // just skip images
         } else if (!src) {
           img.remove(); // src that exists but is null is suspicious
-        } else if (imgHandling === 'IMG-TO-LINK') {
-          // replace images with a link that points to that image
-          if (src.startsWith('data:image/')) {
-            const title = img.getAttribute('title');
-            img.removeAttribute('src');
-            const a = document.createElement('a');
-            a.href = src;
-            a.className = 'image_src_link';
-            a.target = '_blank';
-            a.innerText = title || 'show image';
-            const heightWidth = `height: ${img.clientHeight ? `${Number(img.clientHeight)}px` : 'auto'}; width: ${
-              img.clientWidth ? `${Number(img.clientWidth)}px` : 'auto'
-            };max-width:98%;`;
-            a.setAttribute(
-              'style',
-              `text-decoration: none; background: #FAFAFA; padding: 4px; border: 1px dotted #CACACA; display: inline-block; ${heightWidth}`
-            );
-            a.setAttribute('data-test', 'show-inline-image');
-            Xss.replaceElementDANGEROUSLY(img, a.outerHTML); // xss-safe-value - "a" was build using dom node api
-          } else {
-            const remoteImgEl = `<div class="remote_image_container" data-src="${src}" data-test="remote-image-container"><span>Authenticity of this remote image cannot be verified.</span></div>`;
-            Xss.replaceElementDANGEROUSLY(img, remoteImgEl); // xss-safe-value
-          }
+        } else if (imgHandling === 'IMG-KEEP' && checkValidURL(src)) {
+          // replace remote image with remote_image_container
+          const remoteImgEl = `<div class="remote_image_container" data-src="${src}" data-test="remote-image-container"><span>Authenticity of this remote image cannot be verified.</span></div>`;
+          Xss.replaceElementDANGEROUSLY(img, remoteImgEl); // xss-safe-value
         }
       }
-      if (node.classList.contains('remote_image_container') && imgHandling === 'IMG-TO-PLAIN-URL') {
-        Xss.replaceElementDANGEROUSLY(node, node.getAttribute('data-src') ?? ''); // xss-safe-value
+      if ((node.classList.contains('remote_image_container') || CID_PATTERN.test(node.getAttribute('src') ?? '')) && imgHandling === 'IMG-TO-PLAIN-TEXT') {
+        Xss.replaceElementDANGEROUSLY(node, node.getAttribute('data-src') ?? node.getAttribute('alt') ?? ''); // xss-safe-value
       }
       if ('target' in node) {
         // open links in new window
@@ -200,7 +176,7 @@ export class Xss {
    */
   public static htmlSanitizeAndStripAllTags = (dirtyHtml: string, outputNl: string, trim = true): string => {
     Xss.throwIfNotSupported();
-    let html = Xss.htmlSanitizeKeepBasicTags(dirtyHtml, 'IMG-TO-PLAIN-URL');
+    let html = Xss.htmlSanitizeKeepBasicTags(dirtyHtml, 'IMG-TO-PLAIN-TEXT');
     const random = Str.sloppyRandom(5);
     const br = `CU_BR_${random}`;
     const blockStart = `CU_BS_${random}`;

test/source/mock/google/exported-messages/message-export-186eed032659ad4f.json

@@ -1,6 +1,6 @@
 /* ©️ 2016 - present FlowCrypt a.s. Limitations apply. Contact [email protected] */
 
-export type SanitizeImgHandling = 'IMG-DEL' | 'IMG-KEEP' | 'IMG-TO-LINK';
+export type SanitizeImgHandling = 'IMG-DEL' | 'IMG-KEEP';
 
 /**
  * Look at https://github.com/FlowCrypt/flowcrypt-mobile-core/blob/master/TypeScript/source/platform/xss.ts if node implementation is ever needed for tests.

test/source/platform/xss.ts

@@ -2706,10 +2706,6 @@ const sendImgAndVerifyPresentInSentMsg = async (t: AvaContext, browser: BrowserH
   // open a page with the sent msg, investigate img
   const pgpHostPage = await browser.newPage(t, url);
   const pgpBlockPage = await pgpHostPage.getFrame(['pgp_block.htm']);
-  await pgpBlockPage.waitAll('.image_src_link');
-  expect(await pgpBlockPage.read('.image_src_link')).to.contain('show image');
-  await pgpBlockPage.waitAndClick('.image_src_link');
-  await pgpBlockPage.waitTillGone('.image_src_link');
   const img = await pgpBlockPage.waitAny('body img');
   expect(await PageRecipe.getElementPropertyJson(img, 'src')).to.eq(imgBase64);
 };

test/source/tests/compose.ts

@@ -62,7 +62,8 @@ export const defineDecryptTests = (testVariant: TestVariant, testWithBrowser: Te
         await inboxPage.waitAll('iframe');
         const pgpBlock = await inboxPage.getFrame(['pgp_block.htm']);
         await pgpBlock.waitForContent('@pgp-block-content', 'This message contains inline base64 image');
-        await pgpBlock.waitAndClick('@show-inline-image');
+        await pgpBlock.waitAll('#pgp_block img');
+        await pgpBlock.checkIfImageIsDisplayedCorrectly('#pgp_block img');
         await inboxPage.close();
       })
     );
@@ -679,6 +680,22 @@ XZ8r4OC6sguP/yozWlkG+7dDxsgKQVBENeG6Lw==
       })
     );
 
+    test(
+      'decrypt - display email with cid image correctly',
+      testWithBrowser('compatibility', async (t, browser) => {
+        const threadId = '186eed032659ad4f';
+        const acctEmail = '[email protected]';
+        const inboxPage = await browser.newExtensionPage(t, `chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}&threadId=${threadId}`);
+        await inboxPage.waitAll('iframe');
+        const pgpBlock = await inboxPage.getFrame(['pgp_block.htm']);
+        await pgpBlock.waitForSelTestState('ready');
+        await pgpBlock.checkIfImageIsDisplayedCorrectly('#pgp_block img');
+        const replyFrame = await inboxPage.getFrame(['compose.htm']);
+        await replyFrame.waitAndClick('@action-forward');
+        await replyFrame.waitForContent('@input-body', 'googlelogo_color_272x92dp.png'); // check if forwarded content contains cid image name
+      })
+    );
+
     test(
       "decrypt - thunderbird - signedHtml verifyDetached doesn't duplicate PGP key section",
       testWithBrowser('compatibility', async (t, browser) => {