Harden render iframe open-link handling (#36811)

This PR hardens the handling of the “open-link” action in render iframes
(external rendering iframes). It prevents iframes from triggering unsafe
or unintended redirects or opening new windows via postMessage.

Additionally, it improves iframe height reporting to reduce scrollbar
and height mismatch issues, and adds unit test coverage.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

web_src/js/markup/render-iframe.test.ts

@@ -0,0 +1,46 @@
+import {navigateToIframeLink} from './render-iframe.ts';
+
+describe('navigateToIframeLink', () => {
+  const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
+  const assignSpy = vi.spyOn(window.location, 'assign').mockImplementation(() => undefined);
+
+  test('safe links', () => {
+    navigateToIframeLink('http://example.com', '_blank');
+    expect(openSpy).toHaveBeenCalledWith('http://example.com/', '_blank', 'noopener,noreferrer');
+    vi.clearAllMocks();
+
+    navigateToIframeLink('https://example.com', '_self');
+    expect(assignSpy).toHaveBeenCalledWith('https://example.com/');
+    vi.clearAllMocks();
+
+    navigateToIframeLink('https://example.com', null);
+    expect(assignSpy).toHaveBeenCalledWith('https://example.com/');
+    vi.clearAllMocks();
+
+    navigateToIframeLink('/path', '');
+    expect(assignSpy).toHaveBeenCalledWith('http://localhost:3000/path');
+    vi.clearAllMocks();
+
+    // input can be any type & any value, keep the same behavior as `window.location.href = 0`
+    navigateToIframeLink(0, {});
+    expect(assignSpy).toHaveBeenCalledWith('http://localhost:3000/0');
+    vi.clearAllMocks();
+  });
+
+  test('unsafe links', () => {
+    window.location.href = 'http://localhost:3000/';
+
+    // eslint-disable-next-line no-script-url
+    navigateToIframeLink('javascript:void(0);', '_blank');
+    expect(openSpy).toHaveBeenCalledTimes(0);
+    expect(assignSpy).toHaveBeenCalledTimes(0);
+    expect(window.location.href).toBe('http://localhost:3000/');
+    vi.clearAllMocks();
+
+    navigateToIframeLink('data:image/svg+xml;utf8,<svg></svg>', '');
+    expect(openSpy).toHaveBeenCalledTimes(0);
+    expect(assignSpy).toHaveBeenCalledTimes(0);
+    expect(window.location.href).toBe('http://localhost:3000/');
+    vi.clearAllMocks();
+  });
+});

web_src/js/markup/render-iframe.ts

@@ -1,23 +1,46 @@
 import {generateElemId, queryElemChildren} from '../utils/dom.ts';
 import {isDarkTheme} from '../utils.ts';
 
+function safeRenderIframeLink(link: any): string | null {
+  try {
+    const url = new URL(`${link}`, window.location.href);
+    if (url.protocol !== 'http:' && url.protocol !== 'https:') {
+      console.error(`Unsupported link protocol: ${link}`);
+      return null;
+    }
+    return url.href;
+  } catch (e) {
+    console.error(`Failed to parse link: ${link}, error: ${e}`);
+    return null;
+  }
+}
+
+// This function is only designed for "open-link" command from iframe, is not suitable for other contexts.
+// Because other link protocols are directly handled by the iframe, but not here.
+// Arguments can be any type & any value, they are from "message" event's data which is not controlled by us.
+export function navigateToIframeLink(unsafeLink: any, target: any) {
+  const linkHref = safeRenderIframeLink(unsafeLink);
+  if (linkHref === null) return;
+  if (target === '_blank') {
+    window.open(linkHref, '_blank', 'noopener,noreferrer');
+    return;
+  }
+  // treat all other targets including ("_top", "_self", etc.) as same tab navigation
+  window.location.assign(linkHref);
+}
+
 async function loadRenderIframeContent(iframe: HTMLIFrameElement) {
   const iframeSrcUrl = iframe.getAttribute('data-src')!;
   if (!iframe.id) iframe.id = generateElemId('gitea-iframe-');
 
   window.addEventListener('message', (e) => {
+    if (e.source !== iframe.contentWindow) return;
     if (!e.data?.giteaIframeCmd || e.data?.giteaIframeId !== iframe.id) return;
     const cmd = e.data.giteaIframeCmd;
     if (cmd === 'resize') {
-      // TODO: sometimes the reported iframeHeight is not the size we need, need to figure why. Example: openapi swagger.
-      //  As a workaround, add some pixels here.
-      iframe.style.height = `${e.data.iframeHeight + 2}px`;
+      iframe.style.height = `${e.data.iframeHeight}px`;
     } else if (cmd === 'open-link') {
-      if (e.data.anchorTarget === '_blank') {
-        window.open(e.data.openLink, '_blank');
-      } else {
-        window.location.href = e.data.openLink;
-      }
+      navigateToIframeLink(e.data.openLink, e.data.anchorTarget);
     } else {
       throw new Error(`Unknown gitea iframe cmd: ${cmd}`);
     }

web_src/js/standalone/external-render-iframe.ts

@@ -20,7 +20,15 @@ function mainExternalRenderIframe() {
     window.parent.postMessage({giteaIframeCmd: cmd, giteaIframeId: iframeId, ...data}, '*');
   };
 
-  const updateIframeHeight = () => postIframeMsg('resize', {iframeHeight: document.documentElement.scrollHeight});
+  const updateIframeHeight = () => {
+    // Don't use integer heights from the DOM node.
+    // Use getBoundingClientRect(), then ceil the height to avoid fractional pixels which causes incorrect scrollbars.
+    const rect = document.documentElement.getBoundingClientRect();
+    postIframeMsg('resize', {iframeHeight: Math.ceil(rect.height)});
+    // As long as the parent page is responsible for the iframe height, the iframe itself doesn't need scrollbars.
+    // This style should only be dynamically set here when our code can run.
+    document.documentElement.style.overflowY = 'hidden';
+  };
   const resizeObserver = new ResizeObserver(() => updateIframeHeight());
   resizeObserver.observe(window.document.documentElement);
 
@@ -29,16 +37,18 @@ function mainExternalRenderIframe() {
   // the easiest way to handle dynamic content changes and easy to debug, can be fine-tuned in the future
   setInterval(updateIframeHeight, 1000);
 
-  //  no way to open an absolute link with CSP frame-src, it also needs some tricks like "postMessage" or "copy the link to clipboard"
-  const openIframeLink = (link: string, target: string) => postIframeMsg('open-link', {openLink: link, anchorTarget: target});
+  // no way to open an absolute link with CSP frame-src, it needs some tricks like "postMessage" (let parent window to handle) or "copy the link to clipboard" (let users manually paste it to open).
+  // here we choose "postMessage" way for better user experience.
+  const openIframeLink = (link: string, target: string | null) => postIframeMsg('open-link', {openLink: link, anchorTarget: target});
   document.addEventListener('click', (e) => {
     const el = e.target as HTMLAnchorElement;
     if (el.nodeName !== 'A') return;
-    const href = el.getAttribute('href') || '';
+    const href = el.getAttribute('href') ?? '';
     // safe links: "./any", "../any", "/any", "//host/any", "http://host/any", "https://host/any"
     if (href.startsWith('.') || href.startsWith('/') || href.startsWith('http://') || href.startsWith('https://')) {
       e.preventDefault();
-      openIframeLink(href, el.getAttribute('target')!);
+      const forceTarget = (e.metaKey || e.ctrlKey) ? '_blank' : null;
+      openIframeLink(href, forceTarget ?? el.getAttribute('target'));
     }
   });
 }