Skip to content

Commit 0ffa3cc

Browse files
authored
release: fixes
- Fixed scheduled imports from remote CSV files so charts continue to refresh automatically in the background. - Fixed an issue where charts using lazy rendering could remain blank when their scripts finished loading in the wrong order. - Improved the overall security of the product. Thanks to **creeper_kirby** for responsibly reporting the issue. - Updated dependencies.
2 parents 72ebc74 + efded60 commit 0ffa3cc

32 files changed

Lines changed: 2863 additions & 244 deletions

‎.github/workflows/pr-announcer-docs.yml‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎.wp-env.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
"."
66
],
77
"themes": [],
8+
"mappings": {
9+
"wp-content/mu-plugins/visualizer-e2e-force-lazy-render.php": "./tests/e2e/config/force-lazy-render.php"
10+
},
811
"config": {
912
"WP_DEBUG": true,
1013
"WP_DEBUG_LOG": true,

‎classes/Visualizer/D3Renderer/src/index.js‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,22 @@ function ensurePngName( name ) {
1717
return name.toLowerCase().endsWith( '.png' ) ? name : `${ name }.png`;
1818
}
1919

20+
/**
21+
* The image is produced inside a null-origin sandboxed iframe and returned over
22+
* postMessage, so its `dataUrl` is untrusted. Only accept base64 image data URIs
23+
* before it is opened, downloaded, or rendered; anything else could smuggle
24+
* markup/script into the same-origin popup or an unexpected navigation target.
25+
*
26+
* @param {*} dataUrl Value received from the iframe.
27+
* @return {boolean} Whether the value is a safe base64 image data URI.
28+
*/
29+
function isSafeImageDataUrl( dataUrl ) {
30+
return (
31+
typeof dataUrl === 'string' &&
32+
/^data:image\/(png|jpeg|webp);base64,[a-z0-9+/]+=*$/i.test( dataUrl )
33+
);
34+
}
35+
2036
function downloadDataUrl( dataUrl, name ) {
2137
const link = document.createElement( 'a' );
2238
link.href = dataUrl;
@@ -110,11 +126,16 @@ function handleImageAction( id, name, action ) {
110126
window.removeEventListener( 'message', onResult );
111127

112128
const dataUrl = msg.dataUrl;
113-
if ( ! dataUrl ) return;
129+
if ( ! isSafeImageDataUrl( dataUrl ) ) return;
114130

115131
if ( action === 'print' ) {
116132
const win = window.open();
117-
win.document.write( "<br><img src='" + dataUrl + "'/>" );
133+
if ( ! win ) return;
134+
// Build the node via the DOM API so the untrusted data URI is only
135+
// ever an attribute value, never parsed as markup.
136+
const img = win.document.createElement( 'img' );
137+
img.src = dataUrl;
138+
win.document.body.appendChild( img );
118139
win.document.close();
119140
win.onload = function () { win.print(); setTimeout( win.close, 500 ); };
120141
} else {
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* Security regression: D3 renderer "print"/"image" action must not let a
3+
* compromised chart iframe break out of its sandbox.
4+
*
5+
* D3 chart code runs inside <iframe sandbox="allow-scripts"> (null origin) and
6+
* returns the exported image to the parent over postMessage. That `dataUrl` is
7+
* therefore attacker-controlled. Previously index.js wrote it unescaped into a
8+
* freshly opened, SAME-ORIGIN popup:
9+
*
10+
* const win = window.open(); // about:blank => site origin
11+
* win.document.write( "<br><img src='" + dataUrl + "'/>" );
12+
*
13+
* so a hostile "dataUrl" could inject active markup (e.g. <img onerror=...>) into
14+
* the site's own origin. A Contributor (edit_post on their own draft chart, no
15+
* unfiltered_html) could store such chart code -> stored-XSS privilege escalation.
16+
*
17+
* The handler now validates the value with isSafeImageDataUrl() and builds the
18+
* <img> via the DOM API instead of string concatenation. This test drives the
19+
* REAL index.js module and asserts the breakout is blocked while a legitimate
20+
* export still renders.
21+
*
22+
* @jest-environment jsdom
23+
*/
24+
25+
/* eslint-disable no-undef */
26+
27+
const path = require( 'path' );
28+
29+
describe( 'D3 renderer print/image action', () => {
30+
let actionHandlers;
31+
let openSpy;
32+
33+
beforeEach( () => {
34+
jest.resetModules();
35+
actionHandlers = {};
36+
37+
// Minimal jQuery shim: index.js only uses `$( 'body' ).on( event, fn )`.
38+
global.jQuery = () => ( {
39+
on( event, fn ) {
40+
( actionHandlers[ event ] = actionHandlers[ event ] || [] ).push( fn );
41+
return this;
42+
},
43+
} );
44+
45+
// window.open() returns a popup backed by a real (detached) document so
46+
// createElement/appendChild/write behave exactly as in a browser.
47+
openSpy = jest.spyOn( window, 'open' ).mockImplementation( () => {
48+
const popupDoc = document.implementation.createHTMLDocument( '' );
49+
return { document: popupDoc, print() {}, close() {} };
50+
} );
51+
52+
// Load the real module (registers the body event handlers via the shim).
53+
require( path.resolve( __dirname, '../src/index.js' ) );
54+
} );
55+
56+
afterEach( () => {
57+
openSpy.mockRestore();
58+
} );
59+
60+
/**
61+
* Stub the container/iframe lookups the code performs, with a MALICIOUS
62+
* iframe contentWindow that answers 'export-image' with an attacker-chosen
63+
* dataUrl. Real <iframe> nodes are avoided so jsdom creates no browsing
64+
* contexts; we only satisfy getElementById()/querySelector().
65+
*
66+
* @param {string} id Container id.
67+
* @param {string} dataUrl The value the compromised iframe returns.
68+
*/
69+
function setupChart( id, dataUrl ) {
70+
const evilContentWindow = {
71+
postMessage( msg ) {
72+
if ( ! msg || msg.type !== 'export-image' ) return;
73+
const reply = new window.MessageEvent( 'message', {
74+
data: { type: 'export-image-result', dataUrl },
75+
} );
76+
Object.defineProperty( reply, 'source', { value: evilContentWindow } );
77+
window.dispatchEvent( reply );
78+
},
79+
};
80+
81+
const fakeIframe = { contentWindow: evilContentWindow };
82+
const fakeContainer = {
83+
querySelector: ( sel ) => ( sel.indexOf( 'iframe' ) !== -1 ? fakeIframe : null ),
84+
};
85+
86+
jest.spyOn( document, 'getElementById' ).mockImplementation( ( wanted ) =>
87+
wanted === id ? fakeContainer : null
88+
);
89+
}
90+
91+
function firePrint( id ) {
92+
actionHandlers[ 'visualizer:action:specificchart' ].forEach( ( fn ) =>
93+
fn( {}, { action: 'print', id, dataObj: { name: 'chart' } } )
94+
);
95+
}
96+
97+
it( 'blocks a hostile dataUrl: no popup, no injected markup', () => {
98+
const payload = "x'/><img src=z onerror=\"window.__xss_fired=true\">";
99+
setupChart( 'viz-evil', payload );
100+
101+
firePrint( 'viz-evil' );
102+
103+
// The value fails validation before window.open(), so no popup is created.
104+
expect( openSpy ).not.toHaveBeenCalled();
105+
expect( window.__xss_fired ).toBeUndefined();
106+
} );
107+
108+
it( 'renders a legitimate image export as a single safe <img>', () => {
109+
setupChart( 'viz-safe', 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUg==' );
110+
111+
firePrint( 'viz-safe' );
112+
113+
expect( openSpy ).toHaveBeenCalledTimes( 1 );
114+
const popupDoc = openSpy.mock.results[ 0 ].value.document;
115+
const imgs = popupDoc.querySelectorAll( 'img' );
116+
expect( imgs.length ).toBe( 1 );
117+
expect( imgs[ 0 ].getAttribute( 'src' ) ).toBe(
118+
'data:image/png;base64,iVBORw0KGgoAAAANSUhEUg=='
119+
);
120+
expect( popupDoc.querySelector( 'img[onerror]' ) ).toBeNull();
121+
} );
122+
} );

‎classes/Visualizer/Gutenberg/Block.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ public function get_visualizer_data( $post ) {
307307
$data['visualizer-settings'] = apply_filters( Visualizer_Plugin::FILTER_GET_CHART_SETTINGS, $data['visualizer-settings'], $post_id, $data['visualizer-chart-type'] );
308308

309309
// handle data filter hooks
310-
$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'] );
310+
$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'] );
311311

312312
// we are going to format only for tabular charts, because we are not sure of the effect on others.
313313
// this is to solve the case where boolean data shows up as all-ticks on gutenberg.

‎classes/Visualizer/Module.php‎

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,27 @@ protected static function numberOfCharts() {
722722
return $q->found_posts;
723723
}
724724

725+
/**
726+
* Checks whether the current user may edit a specific chart.
727+
*
728+
* @param int $chart_id Chart ID.
729+
* @return bool
730+
*/
731+
public static function can_edit_chart( $chart_id ) {
732+
$chart_id = absint( $chart_id );
733+
if ( ! $chart_id ) {
734+
return false;
735+
}
736+
737+
$chart = get_post( $chart_id );
738+
return $chart
739+
&& Visualizer_Plugin::CPT_VISUALIZER === $chart->post_type
740+
&& (
741+
current_user_can( 'edit_post', $chart_id )
742+
|| ( (int) $chart->post_author === get_current_user_id() && current_user_can( 'edit_posts' ) )
743+
);
744+
}
745+
725746
/**
726747
* Checks if the PRO version is active.
727748
*
@@ -778,6 +799,85 @@ final public static function get_features_for_license( $plan ) {
778799
}
779800
}
780801

802+
/**
803+
* Safely unserialize chart/source content, blocking PHP object injection.
804+
*
805+
* Single guarded chokepoint shared by chart/source content sinks so the
806+
* allowed_classes guard cannot be dropped from one call site independently.
807+
*
808+
* @param mixed $content The serialized content (only strings are decoded).
809+
* @return mixed The decoded value (array for valid chart data), or false.
810+
*/
811+
public static function decode_content( $content ) {
812+
if ( ! is_string( $content ) ) {
813+
return false;
814+
}
815+
$value = unserialize( trim( $content ), array( 'allowed_classes' => false ) );
816+
if ( self::contains_references( $value ) ) {
817+
return false;
818+
}
819+
return self::strip_incomplete_objects( $value );
820+
}
821+
822+
/**
823+
* Check decoded arrays for references before recursively processing them.
824+
*
825+
* Cyclic serialized arrays necessarily contain a reference. Rejecting all
826+
* references also prevents shared references from becoming cycles later,
827+
* so strip_incomplete_objects() cannot recurse without terminating.
828+
*
829+
* @param mixed $value The decoded value.
830+
* @return bool Whether the value contains an array reference.
831+
*/
832+
private static function contains_references( $value ) {
833+
if ( ! is_array( $value ) ) {
834+
return false;
835+
}
836+
foreach ( array_keys( $value ) as $key ) {
837+
if ( null !== ReflectionReference::fromArrayElement( $value, $key ) ) {
838+
return true;
839+
}
840+
if ( is_array( $value[ $key ] ) && self::contains_references( $value[ $key ] ) ) {
841+
return true;
842+
}
843+
}
844+
return false;
845+
}
846+
847+
/**
848+
* Remove the __PHP_Incomplete_Class stubs the allowed_classes guard leaves
849+
* behind; they crash map_deep() when the decoded value is written back to
850+
* post meta. Legitimate chart content is nested arrays/scalars only.
851+
*
852+
* @param mixed $value The decoded value.
853+
* @return mixed The value without object stubs; false for a top-level stub.
854+
*/
855+
private static function strip_incomplete_objects( $value ) {
856+
if ( $value instanceof __PHP_Incomplete_Class ) {
857+
return false;
858+
}
859+
if ( is_array( $value ) ) {
860+
foreach ( $value as $key => $item ) {
861+
if ( $item instanceof __PHP_Incomplete_Class ) {
862+
unset( $value[ $key ] );
863+
} elseif ( is_array( $item ) ) {
864+
$value[ $key ] = self::strip_incomplete_objects( $item );
865+
}
866+
}
867+
}
868+
return $value;
869+
}
870+
871+
/**
872+
* Object-injection-safe drop-in for maybe_unserialize().
873+
*
874+
* @param mixed $value Raw meta/content value.
875+
* @return mixed The decoded value for serialized input, the value unchanged otherwise.
876+
*/
877+
public static function maybe_decode_content( $value ) {
878+
return is_serialized( $value ) ? self::decode_content( $value ) : $value;
879+
}
880+
781881
/**
782882
* Gets the chart content after common manipulations.
783883
*/
@@ -793,7 +893,7 @@ function ( $matches ) {
793893
},
794894
$post_content
795895
);
796-
$data = unserialize( $post_content );
896+
$data = self::decode_content( $post_content );
797897
$altered = array();
798898
if ( ! empty( $data ) ) {
799899
foreach ( $data as $index => $array ) {

0 commit comments

Comments
 (0)