Improve HTML content sanitization and fix linting issues

.vscode/settings.json

@@ -2,7 +2,7 @@
   "editor.formatOnSave": false,
   "eslint.validate": ["javascript", "javascriptreact", "typescript"],
   "editor.codeActionsOnSave": {
-    "source.fixAll": "explicit"
+    "source.fixAll.eslint": "explicit"
   },
   "jest.jestCommandLine": "npm test --",
   "jest.autoRun": {

src/Scripts/Strings.ts

@@ -115,14 +115,6 @@ export class Strings {
         return "";
     }
 
-    public static mapValueToURL(text: string): string {
-        try {
-            return ["<a href='", text, "' target='_blank'>", this.htmlEncode(text), "</a>"].join("");
-        } catch {
-            return text;
-        }
-    }
-
     // Join an array with char, dropping empty/missing entries
     public static joinArray(array: (string | number | null)[] | null, char: string): string {
         if (!array) return "";

src/Scripts/row/ArchivedRow.test.ts

@@ -1,33 +1,78 @@
 import { ArchivedRow } from "./ArchivedRow";
+import { Decoder } from "../2047";
 import { Strings } from "../Strings";
 
-jest.mock("../Strings", () => ({
-    // eslint-disable-next-line @typescript-eslint/naming-convention
-    Strings: {
-        mapHeaderToURL: jest.fn(),
-        mapValueToURL: jest.fn()
-    }
-}));
+jest.mock("../Strings", () => {
+    const actualStrings = jest.requireActual("../Strings") as typeof import("../Strings");
+
+    return {
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        Strings: {
+            mapHeaderToURL: jest.fn((headerName: string, text?: string) => actualStrings.Strings.mapHeaderToURL(headerName, text)),
+            htmlEncode: jest.fn((value: string) => actualStrings.Strings.htmlEncode(value))
+        }
+    };
+});
 
 describe("ArchivedRow", () => {
-    const header = "testHeader";
+    const header = "Archived-At";
     const label = "testLabel";
     let archivedRow: ArchivedRow;
 
     beforeEach(() => {
-        (Strings.mapHeaderToURL as jest.Mock).mockReturnValue("mockedHeaderURL");
-        (Strings.mapValueToURL as jest.Mock).mockReturnValue("mockedValueURL");
+        jest.clearAllMocks();
         archivedRow = new ArchivedRow(header, label);
     });
 
     it("should set url using Strings.mapHeaderToURL", () => {
         expect(Strings.mapHeaderToURL).toHaveBeenCalledWith(header, label);
-        expect(archivedRow.url).toBe("mockedHeaderURL");
+        expect(archivedRow.url).toBe("<a href = 'https://tools.ietf.org/html/rfc5064' target = '_blank'>testLabel</a>");
+    });
+
+    it("should return html href for bracketed http(s) links", () => {
+        archivedRow.value = "<https://example.test/path>";
+        const valueUrl = archivedRow.valueUrl;
+
+        expect(valueUrl).toContain("<a");
+        expect(valueUrl).toContain("href=");
+        expect(valueUrl).toContain("https://example.test/path");
+        expect(valueUrl).toContain("target=");
+    });
+
+    it("should html encode non-bracketed values", () => {
+        archivedRow.value = "https://example.test/path";
+        const valueUrl = archivedRow.valueUrl;
+        expect(Strings.htmlEncode).toHaveBeenCalledWith(archivedRow.value);
+        expect(valueUrl).toBe("https://example.test/path");
     });
 
-    it("should return valueUrl using Strings.mapValueToURL", () => {
-        archivedRow["valueInternal"] = "internalValue";
-        expect(archivedRow.valueUrl).toBe("mockedValueURL");
-        expect(Strings.mapValueToURL).toHaveBeenCalledWith("internalValue");
+    it("should html encode raw img payload instead of rendering executable HTML", () => {
+        archivedRow.value = "<img src=x onerror=alert('XSS')>";
+        const valueUrl = archivedRow.valueUrl;
+        expect(Strings.htmlEncode).toHaveBeenCalledWith(archivedRow.value);
+        expect(valueUrl).toContain("&lt;img");
+        expect(valueUrl).toContain("&gt;");
+        expect(valueUrl).not.toContain("<img");
+        expect(valueUrl).not.toContain("<a href=");
+    });
+
+    it("should encode RFC2047-decoded img payload instead of rendering executable HTML", () => {
+        const encodedPayload = "=?UTF-8?B?PGltZyBzcmM9eCBvbmVycm9yPWFsZXJ0KCdYU1MnKT4=?=";
+        archivedRow.value = Decoder.clean2047Encoding(encodedPayload);
+
+        expect(archivedRow.valueUrl).toContain("&lt;img");
+        expect(archivedRow.valueUrl).toContain("&gt;");
+        expect(archivedRow.valueUrl).not.toContain("<img");
+        expect(archivedRow.valueUrl).not.toContain("<a href=");
+    });
+
+    it("should render anchor only for strict angle-bracketed http(s) URL", () => {
+        const url = "https://example.com/test/list/foo-users@lists.example.com/message/mysubject/";
+        archivedRow.value = `<${url}>`;
+        const valueUrl = archivedRow.valueUrl;
+
+        expect(valueUrl).toMatch("<a href=\"");
+        expect(valueUrl).toContain(url);
+        expect(valueUrl).toContain("target=\"_blank\"");
     });
 });

src/Scripts/row/ArchivedRow.ts

@@ -1,10 +1,30 @@
 import { Strings } from "../Strings";
 import { SummaryRow } from "./SummaryRow";
 
+// Handle the "Archived-At" header per https://tools.ietf.org/html/rfc5064
 export class ArchivedRow extends SummaryRow {
     constructor(header: string, label: string) {
         super(header, label);
-        this.url = Strings.mapHeaderToURL(header, label);
     }
-    override get valueUrl(): string { return Strings.mapValueToURL(this.valueInternal); }
-}
+    // Return the URL for the archived message, or the encoded value if it's not a valid URL.
+    override get valueUrl(): string {
+        const match = this.valueInternal.match(/^\s*<([^>]+)>\s*$/);
+        if (match?.[1]) {
+            try {
+                const url = new URL(match[1]);
+                if (url.protocol === "http:" || url.protocol === "https:") {
+                    const a = document.createElement("a");
+                    a.href = url.href;
+                    a.target = "_blank";
+                    a.rel = "noopener noreferrer";
+                    a.textContent = url.href;
+                    return a.outerHTML;
+                }
+            } catch {
+                // Fall through to encoded output.
+            }
+        }
+
+        return Strings.htmlEncode(this.valueInternal);
+    }
+}

src/Scripts/ui/newMobilePaneIosFrame.ts

@@ -22,6 +22,7 @@ import { ReceivedRow } from "../row/ReceivedRow";
 import { Row } from "../row/Row";
 import { SummaryRow } from "../row/SummaryRow";
 import { escapeAndHighlight, getViolationsForRow, highlightHtml } from "../rules/ViolationUtils";
+import { Strings } from "../Strings";
 
 // This is the "new-mobile" UI rendered in newMobilePaneIosFrame.html
 
@@ -103,7 +104,7 @@ function addCalloutEntry(name: string, value: string | number | null, parent: HT
         const template = document.getElementById("popover-entry-template") as HTMLTemplateElement;
         const clone = template.content.cloneNode(true) as DocumentFragment;
         const p = clone.querySelector("p") as HTMLElement;
-        p.innerHTML = "<strong>" + name + ": </strong>" + value;
+        p.innerHTML = "<strong>" + name + ": </strong>" + Strings.htmlEncode(value.toString());
         parent.appendChild(clone);
     }
 }
@@ -234,10 +235,10 @@ function buildReceivedTab(viewModel: HeaderModel): void {
                 timelineTime.textContent = currentTime.format("h:mm:ss");
 
                 const timelineSubtitle = innerClone.querySelector(".timeline-item-subtitle") as HTMLElement;
-                timelineSubtitle.innerHTML = "<strong>From: </strong>" + row.from;
+                timelineSubtitle.innerHTML = "<strong>From: </strong>" + Strings.htmlEncode(row.from.toString());
 
                 const timelineText = innerClone.querySelector(".timeline-item-text") as HTMLElement;
-                timelineText.innerHTML = "<strong>To: </strong>" + row.by;
+                timelineText.innerHTML = "<strong>To: </strong>" + Strings.htmlEncode(row.by.toString());
 
                 currentTimeEntry.appendChild(innerClone);
             } else {
@@ -272,7 +273,7 @@ function buildReceivedTab(viewModel: HeaderModel): void {
                 timelineTime.textContent = entryTime.format("h:mm:ss");
 
                 const timelineSubtitle = innerClone.querySelector(".timeline-item-subtitle") as HTMLElement;
-                timelineSubtitle.innerHTML = "<strong>To: </strong>" + row.by;
+                timelineSubtitle.innerHTML = "<strong>To: </strong>" + Strings.htmlEncode(row.by.toString());
 
                 const delayText = innerClone.querySelector(".delay-text") as HTMLElement;
                 delayText.textContent = row.delay.value !== null ? String(row.delay.value) : "";