fix(trace-viewer): strip event handlers and neutralize IFRAME srcdoc/sandbox in snapshot renderer

packages/isomorphic/trace/snapshotRenderer.ts

@@ -106,26 +106,40 @@ export class SnapshotRenderer {
         // Element node.
         // Note that <noscript> will not be rendered by default in the trace viewer, because
         // JS is enabled. So rename it to <x-noscript>.
-        const nodeName = name === 'NOSCRIPT' ? 'X-NOSCRIPT' : name;
+        const upperName = name.toUpperCase();
+        const nodeName = upperName === 'NOSCRIPT' ? 'X-NOSCRIPT' : name;
         const attrs = Object.entries(nodeAttrs || {});
         result.push('<', nodeName);
         const kCurrentSrcAttribute = '__playwright_current_src__';
-        const isFrame = nodeName === 'IFRAME' || nodeName === 'FRAME';
-        const isAnchor = nodeName === 'A';
-        const isImg = nodeName === 'IMG';
-        const isMeta = nodeName === 'META';
+        const isFrame = upperName === 'IFRAME' || upperName === 'FRAME';
+        const isAnchor = upperName === 'A';
+        const isImg = upperName === 'IMG';
+        const isMeta = upperName === 'META';
         const isImgWithCurrentSrc = isImg && attrs.some(a => a[0] === kCurrentSrcAttribute);
-        const isSourceInsidePictureWithCurrentSrc = nodeName === 'SOURCE' && parentTag === 'PICTURE' && parentAttrs?.some(a => a[0] === kCurrentSrcAttribute);
+        const isSourceInsidePictureWithCurrentSrc = upperName === 'SOURCE' && parentTag === 'PICTURE' && parentAttrs?.some(a => a[0] === kCurrentSrcAttribute);
         // For META, only allow a small whitelist of http-equiv directives so a malicious snapshot
         // cannot navigate the snapshot iframe via e.g. <meta http-equiv="refresh"> or otherwise
         // affect the trace viewer.
         const hasUnsafeHttpEquiv = isMeta && attrs.some(a => a[0].toLowerCase() === 'http-equiv' && !kAllowedMetaHttpEquivs.has(a[1].trim().toLowerCase()));
         for (const [attr, value] of attrs) {
           let attrName = attr;
+          // Strip event handler attributes. The capture side already empties these,
+          // but a crafted trace file could include live handlers (e.g. onerror, onclick).
+          // escapeHTMLAttribute does not help because payloads like "alert(1)" contain
+          // no characters that need escaping.
+          if (attr.toLowerCase().startsWith('on'))
+            continue;
           if (isFrame && attr.toLowerCase() === 'src') {
             // Never set relative URLs as <iframe src> - they start fetching frames immediately.
             attrName = '__playwright_src__';
           }
+          if (isFrame && (attr.toLowerCase() === 'srcdoc' || attr.toLowerCase() === 'sandbox')) {
+            // Neutralize srcdoc (could contain arbitrary HTML/script that executes
+            // automatically) and sandbox (attacker-controlled values could alter
+            // iframe security policy). The capture side already skips these, but a
+            // crafted trace could include them.
+            attrName = '__playwright_' + attr.toLowerCase() + '__';
+          }
           if (isImg && attr === kCurrentSrcAttribute) {
             // Render currentSrc for images, so that trace viewer does not accidentally
             // resolve srcset to a different source.

tests/library/snapshot-renderer.spec.ts

@@ -46,3 +46,50 @@ for (const [name, overrides] of [
     expect(html.match(/<\/script>/g)).toHaveLength(1);
   });
 }
+
+test('snapshot renderer strips event handler attributes', () => {
+  const renderer = new SnapshotRenderer(new LRUCache(1_000_000), [], [makeSnapshot({
+    html: ['HTML', {}, ['BODY', {}, ['IMG', { 'onerror': 'alert(1)', 'src': 'x' }]]],
+  })], [], 0);
+  const { html } = renderer.render();
+  expect(html).not.toContain('onerror="alert(1)"');
+  expect(html).toContain('src="x"');
+});
+
+test('snapshot renderer strips onclick attributes', () => {
+  const renderer = new SnapshotRenderer(new LRUCache(1_000_000), [], [makeSnapshot({
+    html: ['HTML', {}, ['BODY', {}, ['DIV', { 'onClick': 'alert(1)' }, 'click me']]],
+  })], [], 0);
+  const { html } = renderer.render();
+  expect(html).not.toContain('onClick');
+  expect(html).not.toContain('onclick');
+  expect(html).toContain('click me');
+});
+
+test('snapshot renderer neutralizes iframe srcdoc', () => {
+  const renderer = new SnapshotRenderer(new LRUCache(1_000_000), [], [makeSnapshot({
+    html: ['HTML', {}, ['BODY', {}, ['IFRAME', { 'srcdoc': '<script>alert(1)</script>' }]]],
+  })], [], 0);
+  const { html } = renderer.render();
+  expect(html).not.toContain(' srcdoc=');
+  expect(html).toContain('__playwright_srcdoc__');
+});
+
+test('snapshot renderer neutralizes iframe sandbox', () => {
+  const renderer = new SnapshotRenderer(new LRUCache(1_000_000), [], [makeSnapshot({
+    html: ['HTML', {}, ['BODY', {}, ['IFRAME', { 'sandbox': 'allow-scripts allow-top-navigation' }]]],
+  })], [], 0);
+  const { html } = renderer.render();
+  expect(html).not.toContain(' sandbox=');
+  expect(html).toContain('__playwright_sandbox__');
+});
+
+test('snapshot renderer handles case-insensitive iframe tag names', () => {
+  const renderer = new SnapshotRenderer(new LRUCache(1_000_000), [], [makeSnapshot({
+    html: ['HTML', {}, ['BODY', {}, ['iframe', { 'srcdoc': '<script>alert(1)</script>', 'src': 'http://evil.com' }]]],
+  })], [], 0);
+  const { html } = renderer.render();
+  expect(html).not.toContain(' srcdoc=');
+  expect(html).toContain('__playwright_srcdoc__');
+  expect(html).toContain('__playwright_src__');
+});