fix: prevent D3 export action XSS via untrusted dataUrl

classes/Visualizer/D3Renderer/src/index.js

@@ -17,6 +17,22 @@ function ensurePngName( name ) {
 	return name.toLowerCase().endsWith( '.png' ) ? name : `${ name }.png`;
 }
 
+/**
+ * The image is produced inside a null-origin sandboxed iframe and returned over
+ * postMessage, so its `dataUrl` is untrusted. Only accept base64 image data URIs
+ * before it is opened, downloaded, or rendered; anything else could smuggle
+ * markup/script into the same-origin popup or an unexpected navigation target.
+ *
+ * @param {*} dataUrl Value received from the iframe.
+ * @return {boolean} Whether the value is a safe base64 image data URI.
+ */
+function isSafeImageDataUrl( dataUrl ) {
+	return (
+		typeof dataUrl === 'string' &&
+		/^data:image\/(png|jpeg|webp);base64,[a-z0-9+/]+=*$/i.test( dataUrl )
+	);
+}
+
 function downloadDataUrl( dataUrl, name ) {
 	const link = document.createElement( 'a' );
 	link.href = dataUrl;
@@ -110,11 +126,16 @@ function handleImageAction( id, name, action ) {
 		window.removeEventListener( 'message', onResult );
 
 		const dataUrl = msg.dataUrl;
-		if ( ! dataUrl ) return;
+		if ( ! isSafeImageDataUrl( dataUrl ) ) return;
 
 		if ( action === 'print' ) {
 			const win = window.open();
-			win.document.write( "<br><img src='" + dataUrl + "'/>" );
+			if ( ! win ) return;
+			// Build the node via the DOM API so the untrusted data URI is only
+			// ever an attribute value, never parsed as markup.
+			const img = win.document.createElement( 'img' );
+			img.src = dataUrl;
+			win.document.body.appendChild( img );
 			win.document.close();
 			win.onload = function () { win.print(); setTimeout( win.close, 500 ); };
 		} else {

classes/Visualizer/D3Renderer/tests/print-action-xss.test.js

@@ -0,0 +1,122 @@
+/**
+ * Security regression: D3 renderer "print"/"image" action must not let a
+ * compromised chart iframe break out of its sandbox.
+ *
+ * D3 chart code runs inside <iframe sandbox="allow-scripts"> (null origin) and
+ * returns the exported image to the parent over postMessage. That `dataUrl` is
+ * therefore attacker-controlled. Previously index.js wrote it unescaped into a
+ * freshly opened, SAME-ORIGIN popup:
+ *
+ *     const win = window.open();                               // about:blank => site origin
+ *     win.document.write( "<br><img src='" + dataUrl + "'/>" );
+ *
+ * so a hostile "dataUrl" could inject active markup (e.g. <img onerror=...>) into
+ * the site's own origin. A Contributor (edit_post on their own draft chart, no
+ * unfiltered_html) could store such chart code -> stored-XSS privilege escalation.
+ *
+ * The handler now validates the value with isSafeImageDataUrl() and builds the
+ * <img> via the DOM API instead of string concatenation. This test drives the
+ * REAL index.js module and asserts the breakout is blocked while a legitimate
+ * export still renders.
+ *
+ * @jest-environment jsdom
+ */
+
+/* eslint-disable no-undef */
+
+const path = require( 'path' );
+
+describe( 'D3 renderer print/image action', () => {
+	let actionHandlers;
+	let openSpy;
+
+	beforeEach( () => {
+		jest.resetModules();
+		actionHandlers = {};
+
+		// Minimal jQuery shim: index.js only uses `$( 'body' ).on( event, fn )`.
+		global.jQuery = () => ( {
+			on( event, fn ) {
+				( actionHandlers[ event ] = actionHandlers[ event ] || [] ).push( fn );
+				return this;
+			},
+		} );
+
+		// window.open() returns a popup backed by a real (detached) document so
+		// createElement/appendChild/write behave exactly as in a browser.
+		openSpy = jest.spyOn( window, 'open' ).mockImplementation( () => {
+			const popupDoc = document.implementation.createHTMLDocument( '' );
+			return { document: popupDoc, print() {}, close() {} };
+		} );
+
+		// Load the real module (registers the body event handlers via the shim).
+		require( path.resolve( __dirname, '../src/index.js' ) );
+	} );
+
+	afterEach( () => {
+		openSpy.mockRestore();
+	} );
+
+	/**
+	 * Stub the container/iframe lookups the code performs, with a MALICIOUS
+	 * iframe contentWindow that answers 'export-image' with an attacker-chosen
+	 * dataUrl. Real <iframe> nodes are avoided so jsdom creates no browsing
+	 * contexts; we only satisfy getElementById()/querySelector().
+	 *
+	 * @param {string} id      Container id.
+	 * @param {string} dataUrl The value the compromised iframe returns.
+	 */
+	function setupChart( id, dataUrl ) {
+		const evilContentWindow = {
+			postMessage( msg ) {
+				if ( ! msg || msg.type !== 'export-image' ) return;
+				const reply = new window.MessageEvent( 'message', {
+					data: { type: 'export-image-result', dataUrl },
+				} );
+				Object.defineProperty( reply, 'source', { value: evilContentWindow } );
+				window.dispatchEvent( reply );
+			},
+		};
+
+		const fakeIframe = { contentWindow: evilContentWindow };
+		const fakeContainer = {
+			querySelector: ( sel ) => ( sel.indexOf( 'iframe' ) !== -1 ? fakeIframe : null ),
+		};
+
+		jest.spyOn( document, 'getElementById' ).mockImplementation( ( wanted ) =>
+			wanted === id ? fakeContainer : null
+		);
+	}
+
+	function firePrint( id ) {
+		actionHandlers[ 'visualizer:action:specificchart' ].forEach( ( fn ) =>
+			fn( {}, { action: 'print', id, dataObj: { name: 'chart' } } )
+		);
+	}
+
+	it( 'blocks a hostile dataUrl: no popup, no injected markup', () => {
+		const payload = "x'/><img src=z onerror=\"window.__xss_fired=true\">";
+		setupChart( 'viz-evil', payload );
+
+		firePrint( 'viz-evil' );
+
+		// The value fails validation before window.open(), so no popup is created.
+		expect( openSpy ).not.toHaveBeenCalled();
+		expect( window.__xss_fired ).toBeUndefined();
+	} );
+
+	it( 'renders a legitimate image export as a single safe <img>', () => {
+		setupChart( 'viz-safe', 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUg==' );
+
+		firePrint( 'viz-safe' );
+
+		expect( openSpy ).toHaveBeenCalledTimes( 1 );
+		const popupDoc = openSpy.mock.results[ 0 ].value.document;
+		const imgs = popupDoc.querySelectorAll( 'img' );
+		expect( imgs.length ).toBe( 1 );
+		expect( imgs[ 0 ].getAttribute( 'src' ) ).toBe(
+			'data:image/png;base64,iVBORw0KGgoAAAANSUhEUg=='
+		);
+		expect( popupDoc.querySelector( 'img[onerror]' ) ).toBeNull();
+	} );
+} );

classes/Visualizer/Module/Chart.php

@@ -1052,7 +1052,9 @@ private function sanitizeSettings( $post_data ): array {
 			'sanitize_textarea_field'
 		);
 
-		if ( '' !== $chart_img ) {
+		// The value is a client-side canvas export; keep it only when it is a
+		// base64 image data URI so nothing else is ever stored unsanitized.
+		if ( is_string( $chart_img ) && preg_match( '#^data:image/(png|jpeg|webp);base64,[A-Za-z0-9+/ ]+=*$#', $chart_img ) ) {
 			$post_data['chart-img'] = $chart_img;
 		}
 
@@ -1744,7 +1746,7 @@ public function saveFilter() {
 	 * @param string $base64_img Chart image.
 	 * @param int    $chart_id Chart ID.
 	 * @param bool   $save_attachment Save attachment.
-	 * @return attachment ID
+	 * @return int Attachment ID, or 0 when no attachment was saved.
 	 */
 	public function save_chart_image( $base64_img, $chart_id, $save_attachment = true ) {
 		// Delete old chart image.
@@ -1761,9 +1763,13 @@ public function save_chart_image( $base64_img, $chart_id, $save_attachment = tru
 		$upload_dir  = wp_upload_dir();
 		$upload_path = str_replace( '/', DIRECTORY_SEPARATOR, $upload_dir['path'] ) . DIRECTORY_SEPARATOR;
 
-		$img             = str_replace( 'data:image/png;base64,', '', $base64_img );
-		$img             = str_replace( ' ', '+', $img );
-		$decoded         = base64_decode( $img );
+		$img     = str_replace( 'data:image/png;base64,', '', (string) $base64_img );
+		$img     = str_replace( ' ', '+', $img );
+		$decoded = base64_decode( $img, true );
+		// The value comes from an untrusted request; only write real PNG bytes to uploads.
+		if ( false === $decoded || 0 !== strncmp( $decoded, "\x89PNG\r\n\x1a\n", 8 ) ) {
+			return 0;
+		}
 		$filename        = 'visualization-' . $chart_id . '.png';
 		$file_type       = 'image/png';
 		$hashed_filename = $filename;

phpstan-baseline.neon

@@ -1218,24 +1218,6 @@ parameters:
 			count: 1
 			path: classes/Visualizer/Module/Chart.php
 
-		-
-			message: '#^Method Visualizer_Module_Chart\:\:save_chart_image\(\) has invalid return type attachment\.$#'
-			identifier: class.notFound
-			count: 1
-			path: classes/Visualizer/Module/Chart.php
-
-		-
-			message: '#^Method Visualizer_Module_Chart\:\:save_chart_image\(\) should return attachment but returns int\.$#'
-			identifier: return.type
-			count: 1
-			path: classes/Visualizer/Module/Chart.php
-
-		-
-			message: '#^Method Visualizer_Module_Chart\:\:save_chart_image\(\) should return attachment but returns int\<0, max\>\.$#'
-			identifier: return.type
-			count: 1
-			path: classes/Visualizer/Module/Chart.php
-
 		-
 			message: '#^Method Visualizer_Module_Chart\:\:setJsonData\(\) has no return type specified\.$#'
 			identifier: missingType.return

tests/test-sanitize-settings.php

@@ -78,6 +78,37 @@ public function test_empty_chart_img_is_dropped() {
 		$this->assertArrayNotHasKey( 'chart-img', $result );
 	}
 
+	/**
+	 * A chart-img that is not a base64 image data URI is dropped entirely.
+	 */
+	public function test_hostile_chart_img_is_dropped() {
+		foreach ( array(
+			'<script>alert(1)</script>',
+			'data:text/html;base64,PHNjcmlwdD4=',
+			'data:image/svg+xml;base64,PHN2Zz4=',
+			'data:image/png;base64,abc"><script>alert(1)</script>',
+			array( 'nested' => 'data:image/png;base64,iVBORw0KGgo=' ),
+		) as $payload ) {
+			$result = $this->sanitize( array( 'chart-img' => $payload ) );
+			$this->assertArrayNotHasKey( 'chart-img', $result );
+		}
+	}
+
+	/**
+	 * save_chart_image() refuses to write anything whose bytes are not PNG.
+	 */
+	public function test_save_chart_image_rejects_non_png_payloads() {
+		$module = new Visualizer_Module_Chart( Visualizer_Plugin::instance() );
+		$chart  = self::factory()->post->create( array( 'post_type' => Visualizer_Plugin::CPT_VISUALIZER ) );
+		foreach ( array(
+			'data:image/png;base64,' . base64_encode( '<?php echo "pwn"; ?>' ),
+			'data:image/png;base64,%%%not-base64%%%',
+			'plain text',
+		) as $payload ) {
+			$this->assertSame( 0, $module->save_chart_image( $payload, $chart ) );
+		}
+	}
+
 	/**
 	 * Multi-line values keep their newlines (textarea sanitization, not text).
 	 */