Apply html utils sanitiser to embedded page (#33842)

* Apply html utils sanitiser to embedded page

* Write tests

apps/web/src/HtmlUtils.tsx

@@ -89,11 +89,18 @@ export function unicodeToShortcode(char: string): string {
 /*
  * Given an untrusted HTML string, return a React node with an sanitized version
  * of that HTML.
+ * @param insaneHtml - the input to sanitize
+ * @param className - an optional class name to apply to the element
+ * @param sanitizeParams - the params to use for sanitization
  */
-export function sanitizedHtmlNode(insaneHtml: string): ReactNode {
-    const saneHtml = sanitizeHtml(insaneHtml, sanitizeHtmlParams);
+export function sanitizedHtmlNode(
+    insaneHtml: string,
+    className?: string,
+    sanitizeParams = sanitizeHtmlParams,
+): ReactNode {
+    const saneHtml = sanitizeHtml(insaneHtml, sanitizeParams);
 
-    return <div dangerouslySetInnerHTML={{ __html: saneHtml }} dir="auto" />;
+    return <div dangerouslySetInnerHTML={{ __html: saneHtml }} dir="auto" className={className} />;
 }
 
 export function getHtmlText(insaneHtml: string): string {

apps/web/src/components/structures/EmbeddedPage.tsx

@@ -19,6 +19,9 @@ import { MatrixClientPeg } from "../../MatrixClientPeg";
 import MatrixClientContext from "../../contexts/MatrixClientContext";
 import { type ActionPayload } from "../../dispatcher/payloads";
 import { Action } from "../../dispatcher/actions.ts";
+import { sanitizedHtmlNode } from "../../HtmlUtils.tsx";
+import { sanitizeHtmlParams, transformTags } from "../../Linkify.ts";
+import { objectExcluding } from "../../utils/objects.ts";
 
 interface IProps {
     // URL to request embedded page content from
@@ -126,7 +129,15 @@ export default class EmbeddedPage extends React.PureComponent<IProps, IState> {
             [`${className}_loggedIn`]: !!client,
         });
 
-        const content = <div className={`${className}_body`} dangerouslySetInnerHTML={{ __html: this.state.page }} />;
+        const content = sanitizedHtmlNode(this.state.page, `${className}_body`, {
+            ...sanitizeHtmlParams,
+            transformTags: objectExcluding(transformTags, [
+                // Disable the transformer for `img` as it only allows mxc resources
+                "img",
+                // Disable the default transformer as it forbids inline styles
+                "*",
+            ]),
+        });
 
         if (this.props.scrollbar) {
             return <AutoHideScrollbar className={classes}>{content}</AutoHideScrollbar>;

apps/web/test/unit-tests/HtmlUtils-test.tsx

@@ -6,11 +6,11 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import React from "react";
+import React, { type ReactElement } from "react";
 import { render, screen } from "jest-matrix-react";
 import parse from "html-react-parser";
 
-import { bodyToHtml, bodyToNode, formatEmojis, topicToHtml } from "../../src/HtmlUtils";
+import { bodyToHtml, bodyToNode, formatEmojis, sanitizedHtmlNode, topicToHtml } from "../../src/HtmlUtils";
 import SettingsStore from "../../src/settings/SettingsStore";
 import { getMockClientWithEventEmitter } from "../test-utils";
 import { SettingLevel } from "../../src/settings/SettingLevel";
@@ -319,3 +319,12 @@ describe("bodyToNode", () => {
         jest.resetAllMocks();
     });
 });
+
+describe("sanitizedHtmlNode", () => {
+    it("should respect className", () => {
+        const sanitized = sanitizedHtmlNode('<a href="https://google.com">Link</a>', "testClass");
+        const { asFragment, getByText } = render(sanitized as ReactElement);
+        expect(getByText("Link").parentNode).toHaveClass("testClass");
+        expect(asFragment()).toMatchSnapshot();
+    });
+});

apps/web/test/unit-tests/__snapshots__/HtmlUtils-test.tsx.snap

@@ -91,3 +91,20 @@ exports[`bodyToNode should handle inline media when mediaIsVisible is true 1`] =
   </span>
 </DocumentFragment>
 `;
+
+exports[`sanitizedHtmlNode should respect className 1`] = `
+<DocumentFragment>
+  <div
+    class="testClass"
+    dir="auto"
+  >
+    <a
+      href="https://google.com"
+      rel="noreferrer noopener"
+      target="_blank"
+    >
+      Link
+    </a>
+  </div>
+</DocumentFragment>
+`;

apps/web/test/unit-tests/components/views/context_menus/EmbeddedPage-test.tsx

@@ -47,4 +47,13 @@ describe("<EmbeddedPage />", () => {
         const { asFragment } = render(<EmbeddedPage />);
         expect(asFragment()).toMatchSnapshot();
     });
+
+    it("should sanitise input", async () => {
+        fetchMock.get("https://other.page", `<h1>Foo</h1><iframe src="https://home.page" />`);
+
+        const { asFragment } = render(<EmbeddedPage url="https://other.page" />);
+        await expect(screen.findByText("Foo")).resolves.toBeVisible();
+        expect(screen.queryByRole("iframe")).not.toBeInTheDocument();
+        expect(asFragment()).toMatchSnapshot();
+    });
 });

apps/web/test/unit-tests/components/views/context_menus/__snapshots__/EmbeddedPage-test.tsx.snap

@@ -7,11 +7,29 @@ exports[`<EmbeddedPage /> should render nothing if no url given 1`] = `
   >
     <div
       class="undefined_body"
+      dir="auto"
     />
   </div>
 </DocumentFragment>
 `;
 
+exports[`<EmbeddedPage /> should sanitise input 1`] = `
+<DocumentFragment>
+  <div
+    class="undefined_guest"
+  >
+    <div
+      class="undefined_body"
+      dir="auto"
+    >
+      <h1>
+        Foo
+      </h1>
+    </div>
+  </div>
+</DocumentFragment>
+`;
+
 exports[`<EmbeddedPage /> should show error if unable to load 1`] = `
 <DocumentFragment>
   <div
@@ -19,6 +37,7 @@ exports[`<EmbeddedPage /> should show error if unable to load 1`] = `
   >
     <div
       class="undefined_body"
+      dir="auto"
     >
       Couldn't load page
     </div>
@@ -33,6 +52,7 @@ exports[`<EmbeddedPage /> should translate _t strings ["] 1`] = `
   >
     <div
       class="undefined_body"
+      dir="auto"
     >
       <h1>
         Przeglądaj pokoje
@@ -49,6 +69,7 @@ exports[`<EmbeddedPage /> should translate _t strings [&#27;] 1`] = `
   >
     <div
       class="undefined_body"
+      dir="auto"
     >
       <h1>
         Przeglądaj pokoje
@@ -65,6 +86,7 @@ exports[`<EmbeddedPage /> should translate _t strings [&#34;] 1`] = `
   >
     <div
       class="undefined_body"
+      dir="auto"
     >
       <h1>
         Przeglądaj pokoje
@@ -81,6 +103,7 @@ exports[`<EmbeddedPage /> should translate _t strings ['] 1`] = `
   >
     <div
       class="undefined_body"
+      dir="auto"
     >
       <h1>
         Przeglądaj pokoje