Skip to content
Open

Release #1340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .wp-env.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"."
],
"themes": [],
"mappings": {
"wp-content/mu-plugins/visualizer-e2e-force-lazy-render.php": "./tests/e2e/config/force-lazy-render.php"
},
"config": {
"WP_DEBUG": true,
"WP_DEBUG_LOG": true,
Expand Down
25 changes: 23 additions & 2 deletions classes/Visualizer/D3Renderer/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
122 changes: 122 additions & 0 deletions classes/Visualizer/D3Renderer/tests/print-action-xss.test.js
Original file line number Diff line number Diff line change
@@ -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();
} );
} );
2 changes: 1 addition & 1 deletion classes/Visualizer/Gutenberg/Block.php
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public function get_visualizer_data( $post ) {
$data['visualizer-settings'] = apply_filters( Visualizer_Plugin::FILTER_GET_CHART_SETTINGS, $data['visualizer-settings'], $post_id, $data['visualizer-chart-type'] );

// handle data filter hooks
$data['visualizer-data'] = apply_filters( Visualizer_Plugin::FILTER_GET_CHART_DATA, unserialize( html_entity_decode( get_the_content( $post_id ) ) ), $post_id, $data['visualizer-chart-type'] );
$data['visualizer-data'] = apply_filters( Visualizer_Plugin::FILTER_GET_CHART_DATA, Visualizer_Module::decode_content( html_entity_decode( get_post_field( 'post_content', $post_id, 'raw' ) ) ), $post_id, $data['visualizer-chart-type'] );

// we are going to format only for tabular charts, because we are not sure of the effect on others.
// this is to solve the case where boolean data shows up as all-ticks on gutenberg.
Expand Down
102 changes: 101 additions & 1 deletion classes/Visualizer/Module.php
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,27 @@ protected static function numberOfCharts() {
return $q->found_posts;
}

/**
* Checks whether the current user may edit a specific chart.
*
* @param int $chart_id Chart ID.
* @return bool
*/
public static function can_edit_chart( $chart_id ) {
$chart_id = absint( $chart_id );
if ( ! $chart_id ) {
return false;
}

$chart = get_post( $chart_id );
return $chart
&& Visualizer_Plugin::CPT_VISUALIZER === $chart->post_type
&& (
current_user_can( 'edit_post', $chart_id )
|| ( (int) $chart->post_author === get_current_user_id() && current_user_can( 'edit_posts' ) )
);
}

/**
* Checks if the PRO version is active.
*
Expand Down Expand Up @@ -778,6 +799,85 @@ final public static function get_features_for_license( $plan ) {
}
}

/**
* Safely unserialize chart/source content, blocking PHP object injection.
*
* Single guarded chokepoint shared by chart/source content sinks so the
* allowed_classes guard cannot be dropped from one call site independently.
*
* @param mixed $content The serialized content (only strings are decoded).
* @return mixed The decoded value (array for valid chart data), or false.
*/
public static function decode_content( $content ) {
if ( ! is_string( $content ) ) {
return false;
}
$value = unserialize( trim( $content ), array( 'allowed_classes' => false ) );
if ( self::contains_references( $value ) ) {
return false;
}
return self::strip_incomplete_objects( $value );
}

/**
* Check decoded arrays for references before recursively processing them.
*
* Cyclic serialized arrays necessarily contain a reference. Rejecting all
* references also prevents shared references from becoming cycles later,
* so strip_incomplete_objects() cannot recurse without terminating.
*
* @param mixed $value The decoded value.
* @return bool Whether the value contains an array reference.
*/
private static function contains_references( $value ) {
if ( ! is_array( $value ) ) {
return false;
}
foreach ( array_keys( $value ) as $key ) {
if ( null !== ReflectionReference::fromArrayElement( $value, $key ) ) {
return true;
}
if ( is_array( $value[ $key ] ) && self::contains_references( $value[ $key ] ) ) {
return true;
}
}
return false;
}

/**
* Remove the __PHP_Incomplete_Class stubs the allowed_classes guard leaves
* behind; they crash map_deep() when the decoded value is written back to
* post meta. Legitimate chart content is nested arrays/scalars only.
*
* @param mixed $value The decoded value.
* @return mixed The value without object stubs; false for a top-level stub.
*/
private static function strip_incomplete_objects( $value ) {
if ( $value instanceof __PHP_Incomplete_Class ) {
return false;
}
if ( is_array( $value ) ) {
foreach ( $value as $key => $item ) {
if ( $item instanceof __PHP_Incomplete_Class ) {
unset( $value[ $key ] );
} elseif ( is_array( $item ) ) {
$value[ $key ] = self::strip_incomplete_objects( $item );
}
}
}
return $value;
}

/**
* Object-injection-safe drop-in for maybe_unserialize().
*
* @param mixed $value Raw meta/content value.
* @return mixed The decoded value for serialized input, the value unchanged otherwise.
*/
public static function maybe_decode_content( $value ) {
return is_serialized( $value ) ? self::decode_content( $value ) : $value;
}

/**
* Gets the chart content after common manipulations.
*/
Expand All @@ -793,7 +893,7 @@ function ( $matches ) {
},
$post_content
);
$data = unserialize( $post_content );
$data = self::decode_content( $post_content );
$altered = array();
if ( ! empty( $data ) ) {
foreach ( $data as $index => $array ) {
Expand Down
Loading
Loading