Harden inline creative rendering (#459)

* Harden inline creative rendering

Author: prk-Jr

crates/common/src/auction/formats.rs

@@ -230,15 +230,17 @@ pub fn convert_to_openrtb_response(
         let width = to_openrtb_i32(bid.width, "width", &bid_context);
         let height = to_openrtb_i32(bid.height, "height", &bid_context);
 
-        // Process creative HTML if present - rewrite URLs and return inline
+        // Process creative HTML if present - — sanitize dangerous markup first, then rewrite URLs.
         let creative_html = if let Some(ref raw_creative) = bid.creative {
-            // Rewrite creative HTML with proxy URLs for first-party delivery
-            let rewritten = creative::rewrite_creative_html(settings, raw_creative);
+            let sanitized = creative::sanitize_creative_html(raw_creative);
+            let rewritten = creative::rewrite_creative_html(settings, &sanitized);
 
             log::debug!(
-                "Rewritten creative for auction {} slot {} ({} bytes)",
+                "Processed creative for auction {} slot {} ({} → {} → {} bytes)",
                 auction_request.id,
                 slot_id,
+                raw_creative.len(),
+                sanitized.len(),
                 rewritten.len()
             );
 

crates/common/src/creative.rs

@@ -114,6 +114,11 @@ export function parseAuctionResponse(body: any): AuctionBid[] {
     if (!Array.isArray(sbBids)) continue;
 
     for (const b of sbBids) {
+      // Coerce missing/null adm to '' so AuctionBid.adm is always a string.
+      // The empty-string case is filtered in renderCreativeInline via the
+      // `if (!bid.adm)` guard. The client-side `typeof !== 'string'` check in
+      // sanitizeCreativeHtml is a second line of defense for callers that bypass
+      // parseAuctionResponse and pass untrusted values directly.
       bids.push({
         impid: b.impid ?? '',
         adm: b.adm ?? '',

crates/js/lib/src/core/auction.ts

@@ -6,11 +6,88 @@ import { getUnit, getAllUnits, firstSize } from './registry';
 import NORMALIZE_CSS from './styles/normalize.css?inline';
 import IFRAME_TEMPLATE from './templates/iframe.html?raw';
 
+// Sandbox permissions granted to creative iframes.
+// Notably absent:
+//   allow-scripts, allow-same-origin — prevent JS execution and same-origin
+//     access, which are the primary attack vectors for malicious creatives.
+//   allow-forms — server-side sanitization strips <form> elements, so form
+//     submission from creatives is not a supported use case. Omitting this token
+//     is consistent with that server-side policy and reduces the attack surface.
+const CREATIVE_SANDBOX_TOKENS = [
+  'allow-popups',
+  'allow-popups-to-escape-sandbox',
+  'allow-top-navigation-by-user-activation',
+] as const;
+
+export type CreativeSanitizationRejectionReason = 'empty-after-sanitize' | 'invalid-creative-html';
+
+export type AcceptedCreativeHtml = {
+  kind: 'accepted';
+  originalLength: number;
+  sanitizedHtml: string;
+  // Always equal to originalLength: the client validates type/emptiness only;
+  // server-side sanitization has already run before adm reaches this function.
+  // Retained so both union members of SanitizeCreativeHtmlResult have consistent fields.
+  sanitizedLength: number;
+  // Always 0 for the same reason — no content is removed client-side.
+  removedCount: number;
+};
+
+export type RejectedCreativeHtml = {
+  kind: 'rejected';
+  originalLength: number;
+  // Always equal to originalLength (or 0 for non-string input): no client-side
+  // removal occurs. Retained so both union members of SanitizeCreativeHtmlResult have consistent fields.
+  sanitizedLength: number;
+  // Always 0 — no content is removed client-side.
+  removedCount: number;
+  rejectionReason: CreativeSanitizationRejectionReason;
+};
+
+export type SanitizeCreativeHtmlResult = AcceptedCreativeHtml | RejectedCreativeHtml;
+
 function normalizeId(raw: string): string {
   const s = String(raw ?? '').trim();
   return s.startsWith('#') ? s.slice(1) : s;
 }
 
+// Validate the untrusted creative fragment before embedding it in the sandboxed iframe.
+// Dangerous markup is stripped server-side before adm reaches the client; this function
+// only guards against type errors and empty payloads. As a result, sanitizedLength always
+// equals originalLength and removedCount is always 0 for accepted creatives — these fields
+// exist for structural consistency with the shared result type but carry no signal here.
+export function sanitizeCreativeHtml(creativeHtml: unknown): SanitizeCreativeHtmlResult {
+  if (typeof creativeHtml !== 'string') {
+    return {
+      kind: 'rejected',
+      originalLength: 0,
+      sanitizedLength: 0,
+      removedCount: 0,
+      rejectionReason: 'invalid-creative-html',
+    };
+  }
+
+  const originalLength = creativeHtml.length;
+
+  if (creativeHtml.trim().length === 0) {
+    return {
+      kind: 'rejected',
+      originalLength,
+      sanitizedLength: originalLength,
+      removedCount: 0,
+      rejectionReason: 'empty-after-sanitize',
+    };
+  }
+
+  return {
+    kind: 'accepted',
+    originalLength,
+    sanitizedHtml: creativeHtml,
+    sanitizedLength: originalLength,
+    removedCount: 0,
+  };
+}
+
 // Locate an ad slot element by id, tolerating funky selectors provided by tag managers.
 export function findSlot(id: string): HTMLElement | null {
   const nid = normalizeId(id);
@@ -85,7 +162,7 @@ export function renderAllAdUnits(): void {
 
 type IframeOptions = { name?: string; title?: string; width?: number; height?: number };
 
-// Construct a sandboxed iframe sized for the ad so we can render arbitrary HTML.
+// Construct a sandboxed iframe sized for sanitized, non-executable creative HTML.
 export function createAdIframe(
   container: HTMLElement,
   opts: IframeOptions = {}
@@ -101,16 +178,14 @@ export function createAdIframe(
   iframe.setAttribute('aria-label', 'Advertisement');
   // Sandbox permissions for creatives
   try {
-    iframe.sandbox.add(
-      'allow-forms',
-      'allow-popups',
-      'allow-popups-to-escape-sandbox',
-      'allow-same-origin',
-      'allow-scripts',
-      'allow-top-navigation-by-user-activation'
-    );
+    if (iframe.sandbox && typeof iframe.sandbox.add === 'function') {
+      iframe.sandbox.add(...CREATIVE_SANDBOX_TOKENS);
+    } else {
+      iframe.setAttribute('sandbox', CREATIVE_SANDBOX_TOKENS.join(' '));
+    }
   } catch (err) {
     log.debug('createAdIframe: sandbox add failed', err);
+    iframe.setAttribute('sandbox', CREATIVE_SANDBOX_TOKENS.join(' '));
   }
   // Sizing + style
   const w = Math.max(0, Number(opts.width ?? 0) | 0);
@@ -129,10 +204,10 @@ export function createAdIframe(
   return iframe;
 }
 
-// Build a complete HTML document for a creative, suitable for use with iframe.srcdoc
+// Build a complete HTML document for a sanitized creative fragment, suitable for iframe.srcdoc.
 export function buildCreativeDocument(creativeHtml: string): string {
-  return IFRAME_TEMPLATE.replace('%NORMALIZE_CSS%', NORMALIZE_CSS).replace(
+  return IFRAME_TEMPLATE.replace('%NORMALIZE_CSS%', () => NORMALIZE_CSS).replace(
     '%CREATIVE_HTML%',
-    creativeHtml
+    () => creativeHtml
   );
 }

crates/js/lib/src/core/render.ts

@@ -2,7 +2,7 @@
 import { log } from './log';
 import { collectContext } from './context';
 import { getAllUnits, firstSize } from './registry';
-import { createAdIframe, findSlot, buildCreativeDocument } from './render';
+import { createAdIframe, findSlot, buildCreativeDocument, sanitizeCreativeHtml } from './render';
 import { buildAdRequest, sendAuction } from './auction';
 
 export type RequestAdsCallback = () => void;
@@ -11,6 +11,16 @@ export interface RequestAdsOptions {
   timeout?: number;
 }
 
+type RenderCreativeInlineOptions = {
+  slotId: string;
+  // Accept unknown input here because bidder JSON is untrusted at runtime.
+  creativeHtml: unknown;
+  creativeWidth?: number;
+  creativeHeight?: number;
+  seat: string;
+  creativeId: string;
+};
+
 // Entry point matching Prebid's requestBids signature; uses unified /auction endpoint.
 export function requestAds(
   callbackOrOpts?: RequestAdsCallback | RequestAdsOptions,
@@ -38,9 +48,19 @@ export function requestAds(
       .then((bids) => {
         log.info('requestAds: got bids', { count: bids.length });
         for (const bid of bids) {
-          if (bid.impid && bid.adm) {
-            renderCreativeInline(bid.impid, bid.adm, bid.width, bid.height);
+          if (!bid.impid) continue;
+          if (!bid.adm) {
+            log.debug('requestAds: bid has no adm, skipping', { slotId: bid.impid });
+            continue;
           }
+          renderCreativeInline({
+            slotId: bid.impid,
+            creativeHtml: bid.adm,
+            creativeWidth: bid.width,
+            creativeHeight: bid.height,
+            seat: bid.seat,
+            creativeId: bid.creativeId,
+          });
         }
         log.info('requestAds: rendered creatives from response');
       })
@@ -59,21 +79,35 @@ export function requestAds(
   }
 }
 
-// Render a creative by writing HTML directly into a sandboxed iframe.
-function renderCreativeInline(
-  slotId: string,
-  creativeHtml: string,
-  creativeWidth?: number,
-  creativeHeight?: number
-): void {
+// Render a creative by writing sanitized, non-executable HTML into a sandboxed iframe.
+function renderCreativeInline({
+  slotId,
+  creativeHtml,
+  creativeWidth,
+  creativeHeight,
+  seat,
+  creativeId,
+}: RenderCreativeInlineOptions): void {
   const container = findSlot(slotId) as HTMLElement | null;
   if (!container) {
-    log.warn('renderCreativeInline: slot not found; skipping render', { slotId });
+    log.warn('renderCreativeInline: slot not found; skipping render', { slotId, seat, creativeId });
     return;
   }
 
   try {
-    // Clear previous content
+    const sanitization = sanitizeCreativeHtml(creativeHtml);
+    if (sanitization.kind === 'rejected') {
+      log.warn('renderCreativeInline: rejected creative', {
+        slotId,
+        seat,
+        creativeId,
+        originalLength: sanitization.originalLength,
+        rejectionReason: sanitization.rejectionReason,
+      });
+      return;
+    }
+
+    // Clear the slot only after sanitization succeeds so rejected creatives never blank existing content.
     container.innerHTML = '';
 
     // Determine size with fallback chain: creative size → ad unit size → 300x250
@@ -99,15 +133,17 @@ function renderCreativeInline(
       height,
     });
 
-    iframe.srcdoc = buildCreativeDocument(creativeHtml);
+    iframe.srcdoc = buildCreativeDocument(sanitization.sanitizedHtml);
 
     log.info('renderCreativeInline: rendered', {
       slotId,
+      seat,
+      creativeId,
       width,
       height,
-      htmlLength: creativeHtml.length,
+      originalLength: sanitization.originalLength,
     });
   } catch (err) {
-    log.warn('renderCreativeInline: failed', { slotId, err });
+    log.warn('renderCreativeInline: failed', { slotId, seat, creativeId, err });
   }
 }

crates/js/lib/src/core/request.ts

@@ -6,17 +6,104 @@ describe('render', () => {
     document.body.innerHTML = '';
   });
 
-  it('creates a sandboxed iframe with creative HTML via srcdoc', async () => {
-    const { createAdIframe, buildCreativeDocument } = await import('../../src/core/render');
+  it('creates a sandboxed iframe with sanitized creative HTML via srcdoc', async () => {
+    const { createAdIframe, buildCreativeDocument, sanitizeCreativeHtml } =
+      await import('../../src/core/render');
     const div = document.createElement('div');
     div.id = 'slotA';
     document.body.appendChild(div);
 
     const iframe = createAdIframe(div, { name: 'test', width: 300, height: 250 });
-    iframe.srcdoc = buildCreativeDocument('<span>ad</span>');
+    const sanitization = sanitizeCreativeHtml('<span>ad</span>');
+
+    expect(sanitization.kind).toBe('accepted');
+    if (sanitization.kind !== 'accepted') {
+      throw new Error('should accept safe creative markup');
+    }
+
+    iframe.srcdoc = buildCreativeDocument(sanitization.sanitizedHtml);
 
     expect(iframe).toBeTruthy();
     expect(iframe.srcdoc).toContain('<span>ad</span>');
     expect(div.querySelector('iframe')).toBe(iframe);
+    const sandbox = iframe.getAttribute('sandbox') ?? '';
+    expect(sandbox).not.toContain('allow-forms');
+    expect(sandbox).toContain('allow-popups');
+    expect(sandbox).toContain('allow-popups-to-escape-sandbox');
+    expect(sandbox).toContain('allow-top-navigation-by-user-activation');
+    expect(sandbox).not.toContain('allow-same-origin');
+    expect(sandbox).not.toContain('allow-scripts');
+  });
+
+  it('preserves dollar sequences when building the creative document', async () => {
+    const { buildCreativeDocument } = await import('../../src/core/render');
+    const creativeHtml = "<div>$& $$ $1 $` $'</div>";
+    const documentHtml = buildCreativeDocument(creativeHtml);
+
+    expect(documentHtml).toContain(creativeHtml);
+  });
+
+  it('accepts safe static markup during sanitization', async () => {
+    const { sanitizeCreativeHtml } = await import('../../src/core/render');
+    const sanitization = sanitizeCreativeHtml(
+      '<div><a href="mailto:test@example.com">Contact</a><img src="https://example.com/ad.png" alt="ad creative"></div>'
+    );
+
+    expect(sanitization.kind).toBe('accepted');
+    if (sanitization.kind !== 'accepted') {
+      throw new Error('should accept safe static creative HTML');
+    }
+
+    expect(sanitization.sanitizedHtml).toContain('<img');
+    expect(sanitization.sanitizedHtml).toContain('mailto:test@example.com');
+    expect(sanitization.removedCount).toBe(0);
+  });
+
+  it('accepts safe inline styles during sanitization', async () => {
+    const { sanitizeCreativeHtml } = await import('../../src/core/render');
+    const sanitization = sanitizeCreativeHtml('<div style="color: red">styled creative</div>');
+
+    expect(sanitization.kind).toBe('accepted');
+    if (sanitization.kind !== 'accepted') {
+      throw new Error('should accept safe inline styles');
+    }
+
+    expect(sanitization.sanitizedHtml).toContain('style=');
+    expect(sanitization.removedCount).toBe(0);
+  });
+
+  it('accepts server-sanitized creative HTML (content-based checks are server-side)', async () => {
+    const { sanitizeCreativeHtml } = await import('../../src/core/render');
+    // The server strips dangerous markup before adm reaches the client.
+    // The client only validates type and emptiness — content passes through.
+    const sanitization = sanitizeCreativeHtml(
+      '<div><img src="https://cdn.example.com/ad.png" alt="ad"></div>'
+    );
+
+    expect(sanitization.kind).toBe('accepted');
+  });
+
+  it('rejects malformed non-string creative HTML', async () => {
+    const { sanitizeCreativeHtml } = await import('../../src/core/render');
+    const sanitization = sanitizeCreativeHtml({ html: '<div>bad</div>' });
+
+    expect(sanitization).toEqual(
+      expect.objectContaining({
+        kind: 'rejected',
+        rejectionReason: 'invalid-creative-html',
+      })
+    );
+  });
+
+  it('rejects creatives that sanitize to empty markup', async () => {
+    const { sanitizeCreativeHtml } = await import('../../src/core/render');
+    const sanitization = sanitizeCreativeHtml('   ');
+
+    expect(sanitization).toEqual(
+      expect.objectContaining({
+        kind: 'rejected',
+        rejectionReason: 'empty-after-sanitize',
+      })
+    );
   });
 });