From b8f343b0da323dc7a0f1688d3b310c9293399eff Mon Sep 17 00:00:00 2001 From: lucadobrescu Date: Wed, 8 Jul 2026 14:30:32 +0300 Subject: [PATCH 01/12] fix: preserve script order for lazy-rendered charts so the renderer registers before render-facade fires (#1319) Co-Authored-By: Claude Fable 5 --- classes/Visualizer/Module/Frontend.php | 16 ++++-- tests/e2e/specs/lazy-render.spec.js | 75 ++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/specs/lazy-render.spec.js diff --git a/classes/Visualizer/Module/Frontend.php b/classes/Visualizer/Module/Frontend.php index 1d3bbabd..eca006a9 100644 --- a/classes/Visualizer/Module/Frontend.php +++ b/classes/Visualizer/Module/Frontend.php @@ -803,14 +803,20 @@ function visualizerTriggerScriptLoader() { function visualizerLoadScripts() { document.querySelectorAll("script[data-visualizer-script]").forEach(function(elem) { - jQuery.getScript( elem.getAttribute("data-visualizer-script") ) - .done( function( script, textStatus ) { - elem.setAttribute("src", elem.getAttribute("data-visualizer-script")); - elem.removeAttribute("data-visualizer-script"); + // Replace the placeholder with a real script tag using async=false: + // scripts download in parallel but execute in insertion order, which + // preserves the WordPress dependency order. Parallel jQuery.getScript + // calls could execute render-facade.js before the chart renderer had + // registered its render event listener, leaving charts blank. + var script = document.createElement("script"); + script.src = elem.getAttribute("data-visualizer-script"); + script.async = false; + script.onload = function() { setTimeout( function() { visualizerRefreshChart(); } ); - } ); + }; + elem.parentNode.replaceChild(script, elem); }); } diff --git a/tests/e2e/specs/lazy-render.spec.js b/tests/e2e/specs/lazy-render.spec.js new file mode 100644 index 00000000..0e682c2e --- /dev/null +++ b/tests/e2e/specs/lazy-render.spec.js @@ -0,0 +1,75 @@ +/** + * WordPress dependencies + */ +const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); + +/** + * Internal dependencies + */ +const { deleteAllCharts, createChartWithAdmin } = require( '../utils/common' ); + +/** + * Regression tests for https://github.com/Codeinwp/visualizer/issues/1319 + * + * Charts with lazy rendering enabled load their scripts only after the first + * user interaction, via `script[data-visualizer-script]` placeholders. The + * loader must preserve the WordPress dependency order: if render-facade.js + * executes before the chart renderer (render-google.js etc.) has registered + * its `visualizer:render:chart:start` listener, the render event is lost and + * the chart stays blank forever. + */ +test.describe( 'Lazy rendered charts (frontend)', () => { + test.beforeEach( async ( { requestUtils } ) => { + await deleteAllCharts( requestUtils ); + } ); + + async function createChartOnPost( admin, page, requestUtils ) { + // New charts have lazy rendering enabled by default. + const chartId = await createChartWithAdmin( admin, page ); + + const post = await requestUtils.createPost( { + title: 'Lazy render', + content: `[visualizer id="${ chartId }" lazy="no" class=""]`, + status: 'publish', + } ); + + return { chartId, post }; + } + + async function triggerLazyLoader( page ) { + // Scripts are swapped to `data-visualizer-script` placeholders. + await expect( page.locator( 'script[data-visualizer-script]' ).first() ).toBeAttached(); + + // The loader starts on the first user interaction. + await page.evaluate( () => window.dispatchEvent( new Event( 'scroll' ) ) ); + } + + test( 'chart renders when the renderer script loads after render-facade.js', async ( { admin, page, requestUtils } ) => { + const { chartId, post } = await createChartOnPost( admin, page, requestUtils ); + + // Delay the chart renderer scripts so render-facade.js would win the + // load race — the deterministic reproduction of issue #1319. + await page.route( /js\/render-(google|chartjs|datatables)\.js/, async ( route ) => { + await new Promise( ( resolve ) => setTimeout( resolve, 2000 ) ); + await route.continue(); + } ); + + await page.goto( `/?p=${ post.id }` ); + await triggerLazyLoader( page ); + + const chart = page.locator( `.visualizer-front-${ chartId }` ).first(); + await expect( chart ).toHaveClass( /visualizer-chart-loaded/, { timeout: 15000 } ); + await expect( chart.locator( 'svg, canvas, table' ).first() ).toBeVisible(); + } ); + + test( 'chart renders without artificial script delay', async ( { admin, page, requestUtils } ) => { + const { chartId, post } = await createChartOnPost( admin, page, requestUtils ); + + await page.goto( `/?p=${ post.id }` ); + await triggerLazyLoader( page ); + + const chart = page.locator( `.visualizer-front-${ chartId }` ).first(); + await expect( chart ).toHaveClass( /visualizer-chart-loaded/, { timeout: 15000 } ); + await expect( chart.locator( 'svg, canvas, table' ).first() ).toBeVisible(); + } ); +} ); From f7a18e6f8c06146362730c97c5d5c0a294420d4f Mon Sep 17 00:00:00 2001 From: lucadobrescu Date: Wed, 8 Jul 2026 15:12:19 +0300 Subject: [PATCH 02/12] test: force lazy rendering in e2e env so the lazy-render spec exercises the delayed script loader Co-Authored-By: Claude Fable 5 --- .wp-env.json | 3 +++ tests/e2e/config/force-lazy-render.php | 10 ++++++++++ tests/e2e/specs/lazy-render.spec.js | 3 ++- 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/config/force-lazy-render.php diff --git a/.wp-env.json b/.wp-env.json index 9d19ec37..61758738 100644 --- a/.wp-env.json +++ b/.wp-env.json @@ -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, diff --git a/tests/e2e/config/force-lazy-render.php b/tests/e2e/config/force-lazy-render.php new file mode 100644 index 00000000..2de83bd0 --- /dev/null +++ b/tests/e2e/config/force-lazy-render.php @@ -0,0 +1,10 @@ + { } ); async function createChartOnPost( admin, page, requestUtils ) { - // New charts have lazy rendering enabled by default. + // Lazy rendering is forced on by tests/e2e/config/force-lazy-render.php + // (mapped as an mu-plugin in .wp-env.json). const chartId = await createChartWithAdmin( admin, page ); const post = await requestUtils.createPost( { From 99d4f52eac4545ec0d4cfff9cd52eb15df3dac77 Mon Sep 17 00:00:00 2001 From: Alexia-Soare <108459992+Alexia-Soare@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:03:30 +0300 Subject: [PATCH 03/12] fix: scheduled CSV refresh runs without a logged-in user Background schedule/refresh invokes the upload handler with no logged-in user and VISUALIZER_DO_NOT_DIE set. Since Feb 2026 uploadData() also requires edit_posts/edit_post, so the scheduled import silently returned and charts stopped auto-updating. Gate the two capability checks on $can_die so they apply to real web requests only; trusted internal calls (the only context that defines VISUALIZER_DO_NOT_DIE, never from request input) proceed. Web path and the nonce check are unchanged. Fixes Codeinwp/visualizer-pro#590 Co-Authored-By: Claude Fable 5 --- classes/Visualizer/Module/Chart.php | 7 +++-- tests/test-schedule.php | 45 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 tests/test-schedule.php diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index fc3190f4..340bca54 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -1222,12 +1222,13 @@ public function uploadData() { // if this is being called internally from pro and VISUALIZER_DO_NOT_DIE is set. // otherwise, assume this is a normal web request. $can_die = ! ( defined( 'VISUALIZER_DO_NOT_DIE' ) && VISUALIZER_DO_NOT_DIE ); + // $can_die also gates the capability checks below, so VISUALIZER_DO_NOT_DIE must stay internal-only (never set from request input or globally). - // validate nonce + // validate nonce; capability check applies to web requests only, not trusted internal calls. if ( ! isset( $_GET['nonce'] ) || ! wp_verify_nonce( $_GET['nonce'], 'visualizer-upload-data' ) || - ! current_user_can( 'edit_posts' ) + ( $can_die && ! current_user_can( 'edit_posts' ) ) ) { if ( ! $can_die ) { return; @@ -1244,7 +1245,7 @@ public function uploadData() { ! $chart_id || ! $chart || $chart->post_type !== Visualizer_Plugin::CPT_VISUALIZER || - ! current_user_can( 'edit_post', $chart_id ) + ( $can_die && ! current_user_can( 'edit_post', $chart_id ) ) ) { if ( ! $can_die ) { return; diff --git a/tests/test-schedule.php b/tests/test-schedule.php new file mode 100644 index 00000000..20d57004 --- /dev/null +++ b/tests/test-schedule.php @@ -0,0 +1,45 @@ +assertFalse( current_user_can( 'edit_posts' ), 'precondition: background context has no editing user' ); + + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'publish', + 'post_content' => 'OLD-CONTENT', + ) + ); + + // mimic Pro's internal invocation; clean superglobals so prior tests don't reroute uploadData(). + $_GET = $_POST = $_FILES = array(); + define( 'VISUALIZER_DO_NOT_DIE', true ); + $_GET['nonce'] = wp_create_nonce( 'visualizer-upload-data' ); + $_GET['chart'] = $chart_id; + $_POST['editor-type'] = 'text'; + $_POST['chart_data'] = "Name,Value\nstring,number\nAlpha,111\nBeta,222"; + + do_action( 'wp_ajax_' . Visualizer_Plugin::ACTION_UPLOAD_DATA ); + + $content = get_post_field( 'post_content', $chart_id ); + $this->assertStringContainsString( 'Alpha', $content, 'scheduled/background import must update chart data without a logged-in user' ); + } +} From 41c7ec5b09ac2ba9543dbca903731422824c8d8b Mon Sep 17 00:00:00 2001 From: Soare Robert Daniel Date: Wed, 15 Jul 2026 10:24:47 +0300 Subject: [PATCH 04/12] fix: centralize safe remote imports for SSRF (#1336) --- classes/Visualizer/Module/AIBuilder.php | 15 +- classes/Visualizer/Module/Chart.php | 18 +- classes/Visualizer/Remote/Fetch.php | 379 +++++++++++++++++ classes/Visualizer/Source/Csv.php | 32 +- classes/Visualizer/Source/Csv/Remote.php | 46 +- classes/Visualizer/Source/Json.php | 15 +- classes/Visualizer/Source/Xlsx.php | 7 +- classes/Visualizer/Source/Xlsx/Remote.php | 43 +- tests/e2e/specs/remote-import.spec.js | 70 ++++ tests/test-ajax.php | 126 ++++-- tests/test-remote-fetch.php | 489 ++++++++++++++++++++++ 11 files changed, 1124 insertions(+), 116 deletions(-) create mode 100644 classes/Visualizer/Remote/Fetch.php create mode 100644 tests/e2e/specs/remote-import.spec.js create mode 100644 tests/test-remote-fetch.php diff --git a/classes/Visualizer/Module/AIBuilder.php b/classes/Visualizer/Module/AIBuilder.php index 83334c66..c01087eb 100644 --- a/classes/Visualizer/Module/AIBuilder.php +++ b/classes/Visualizer/Module/AIBuilder.php @@ -206,7 +206,7 @@ public function fetchChart(): void { /** * Determines whether a remote URL serves an XLSX file. * - * Uses wp_safe_remote_get() and checks ZIP magic number (PK\x03\x04). + * Uses the shared remote-fetch policy and checks ZIP magic number (PK\x03\x04). * * @access private * @param string $url The remote URL to probe. @@ -218,14 +218,15 @@ private static function _url_is_xlsx( $url ) { return false; } - $response = wp_safe_remote_get( + $response = Visualizer_Remote_Fetch::request( $url, array( - 'timeout' => 15, - 'redirection' => 5, - 'stream' => true, - 'filename' => $tmpfile, - 'headers' => array( 'Range' => 'bytes=0-3' ), + 'timeout' => 15, + 'redirection' => 5, + 'stream' => true, + 'filename' => $tmpfile, + 'headers' => array( 'Range' => 'bytes=0-3' ), + 'limit_response_size' => 4, ) ); diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index f71f1fc9..0fffdb5b 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -204,7 +204,8 @@ public function getJsonData() { $chart_id = $params['chart']; - if ( empty( $chart_id ) ) { + $chart = $chart_id ? get_post( $chart_id ) : null; + if ( ! $chart || Visualizer_Plugin::CPT_VISUALIZER !== $chart->post_type || ! current_user_can( 'edit_post', $chart_id ) ) { wp_die(); } @@ -1037,7 +1038,7 @@ public function renderFlattrScript() { * Used as a fallback when the URL path has no recognisable file extension * (e.g. SharePoint, signed S3 URLs, or "download?id=…" endpoints). * - * Uses wp_safe_remote_get() to block requests to private/loopback addresses, + * Uses the shared remote-fetch policy to block non-public destinations, * and streams the response to a temp file so no body data is held in memory * regardless of whether the server honours the Range header. * @@ -1056,14 +1057,15 @@ private static function _url_is_xlsx( $url ) { return false; } - $response = wp_safe_remote_get( + $response = Visualizer_Remote_Fetch::request( $url, array( - 'timeout' => 10, - 'user-agent' => 'WordPress/' . get_bloginfo( 'version' ), - 'headers' => array( 'Range' => 'bytes=0-3' ), - 'stream' => true, - 'filename' => $tmpfile, + 'timeout' => 10, + 'user-agent' => 'WordPress/' . get_bloginfo( 'version' ), + 'headers' => array( 'Range' => 'bytes=0-3' ), + 'stream' => true, + 'filename' => $tmpfile, + 'limit_response_size' => 4, ) ); diff --git a/classes/Visualizer/Remote/Fetch.php b/classes/Visualizer/Remote/Fetch.php new file mode 100644 index 00000000..92343580 --- /dev/null +++ b/classes/Visualizer/Remote/Fetch.php @@ -0,0 +1,379 @@ + $args Optional WordPress HTTP arguments. + * @return array|WP_Error + */ + public static function request( $url, $args = array() ) { + $args = wp_parse_args( + $args, + array( + 'method' => 'GET', + 'timeout' => 15, + ) + ); + + $redirects = isset( $args['redirection'] ) ? min( self::MAX_REDIRECTS, max( 0, (int) $args['redirection'] ) ) : self::MAX_REDIRECTS; + unset( $args['redirection'] ); + + $args = self::enforce_request_policy( $args ); + if ( is_wp_error( $args ) ) { + return $args; + } + + for ( $redirect = 0; $redirect <= $redirects; $redirect++ ) { + $ips = array(); + $validated_url = self::validate_url( $url, $ips ); + if ( is_wp_error( $validated_url ) ) { + return $validated_url; + } + + $request_args = $args; + $request_args['redirection'] = 0; + $request_args['reject_unsafe_urls'] = true; + + $pin = self::pin_validated_addresses( $validated_url, $ips ); + if ( is_wp_error( $pin ) ) { + return $pin; + } + $response = wp_safe_remote_request( $validated_url, $request_args ); + if ( $pin ) { + remove_action( 'http_api_curl', $pin ); + } + if ( is_wp_error( $response ) ) { + return $response; + } + + $status = (int) wp_remote_retrieve_response_code( $response ); + $location = wp_remote_retrieve_header( $response, 'location' ); + if ( $status < 300 || $status > 399 || empty( $location ) ) { + return $response; + } + + if ( $redirect === $redirects ) { + return new WP_Error( 'visualizer_too_many_redirects', 'The remote URL redirected too many times.' ); + } + + $next_url = WP_Http::make_absolute_url( $location, $validated_url ); + if ( ! self::same_origin( $validated_url, $next_url ) ) { + $args['headers'] = self::headers_for_cross_origin_redirect( isset( $args['headers'] ) ? $args['headers'] : array() ); + unset( $args['cookies'] ); + } + + if ( 303 === $status || ( in_array( $status, array( 301, 302 ), true ) && 'POST' === $args['method'] ) ) { + $args['method'] = 'GET'; + unset( $args['body'] ); + } + + $url = $next_url; + } + + return new WP_Error( 'visualizer_remote_request', 'The remote request could not be completed.' ); + } + + /** + * Downloads a remote resource to a temporary file. + * + * The caller is responsible for deleting the returned file. + * + * @param string $url Remote URL. + * @param array $args Optional WordPress HTTP arguments. + * @return string|WP_Error + */ + public static function download( $url, $args = array() ) { + require_once ABSPATH . 'wp-admin/includes/file.php'; + + $max_bytes = isset( $args['limit_response_size'] ) ? (int) $args['limit_response_size'] : self::MAX_DOWNLOAD_BYTES; + if ( $max_bytes < 1 ) { + return new WP_Error( 'visualizer_remote_size', 'The remote file size limit is invalid.' ); + } + + $tmpfile = wp_tempnam( (string) wp_parse_url( $url, PHP_URL_PATH ) ); + if ( ! $tmpfile ) { + return new WP_Error( 'visualizer_temp_file', 'Could not create a temporary file.' ); + } + + $args['stream'] = true; + $args['filename'] = $tmpfile; + $args['limit_response_size'] = $max_bytes < PHP_INT_MAX ? $max_bytes + 1 : $max_bytes; + $response = self::request( $url, $args ); + + if ( is_wp_error( $response ) ) { + wp_delete_file( $tmpfile ); + return $response; + } + + if ( 200 !== (int) wp_remote_retrieve_response_code( $response ) ) { + wp_delete_file( $tmpfile ); + return new WP_Error( 'visualizer_remote_status', 'The remote server returned an unexpected response.' ); + } + + // Download one extra byte so an exactly-at-limit file remains valid. + clearstatcache( true, $tmpfile ); + if ( filesize( $tmpfile ) > $max_bytes ) { + wp_delete_file( $tmpfile ); + return new WP_Error( 'visualizer_remote_size', 'The remote file is too large to import.' ); + } + + return $tmpfile; + } + + /** + * Applies method and header restrictions. + * + * @param array $args HTTP arguments. + * @return array|WP_Error + */ + private static function enforce_request_policy( $args ) { + $args['method'] = strtoupper( (string) $args['method'] ); + if ( ! in_array( $args['method'], array( 'GET', 'POST' ), true ) ) { + return new WP_Error( 'visualizer_remote_method', 'Only GET and POST remote requests are allowed.' ); + } + + $blocked_headers = array( 'connection', 'content-length', 'host', 'proxy-authorization', 'proxy-connection', 'te', 'trailer', 'transfer-encoding', 'upgrade' ); + foreach ( isset( $args['headers'] ) ? $args['headers'] : array() as $name => $value ) { + if ( in_array( strtolower( (string) $name ), $blocked_headers, true ) ) { + unset( $args['headers'][ $name ] ); + } + } + + $args['timeout'] = min( 30, max( 1, (int) $args['timeout'] ) ); + + return $args; + } + + /** + * Binds the cURL transport to the addresses that passed validation. + * + * Without this the transport re-resolves the host on connect, letting a + * rebinding nameserver answer with a private address after validation + * passed. Hostname requests fail closed when cURL pinning is unavailable. + * + * @param string $url Validated URL. + * @param string[] $ips Validated addresses. + * @return callable|WP_Error|null The registered hook to remove after dispatch, an error when pinning is unavailable, or null for an IP literal or exempt host. + */ + private static function pin_validated_addresses( $url, $ips ) { + if ( empty( $ips ) ) { + return null; + } + + $parsed = wp_parse_url( $url ); + if ( filter_var( rtrim( $parsed['host'], '.' ), FILTER_VALIDATE_IP ) ) { + return null; + } + + if ( ! function_exists( 'curl_init' ) || ! function_exists( 'curl_exec' ) || ! defined( 'CURLOPT_RESOLVE' ) ) { + return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely on this server.' ); + } + + if ( 'https' === strtolower( $parsed['scheme'] ) ) { + $curl = curl_version(); + if ( empty( $curl['features'] ) || ! defined( 'CURL_VERSION_SSL' ) || ! ( $curl['features'] & CURL_VERSION_SSL ) ) { + return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely on this server.' ); + } + } + + $proxy = new WP_HTTP_Proxy(); + if ( $proxy->is_enabled() && $proxy->send_through_proxy( $url ) ) { + return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely through the configured proxy.' ); + } + + $addresses = array(); + foreach ( $ips as $ip ) { + $addresses[] = filter_var( $ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 ) ? '[' . $ip . ']' : $ip; + } + + $entry = sprintf( '%s:%d:%s', strtolower( rtrim( $parsed['host'], '.' ) ), self::url_port( $parsed ), implode( ',', $addresses ) ); + $pin = function ( $handle ) use ( $entry ) { + curl_setopt( $handle, CURLOPT_RESOLVE, array( $entry ) ); + }; + add_action( 'http_api_curl', $pin ); + + return $pin; + } + + /** + * Validates URL syntax and every address returned by DNS. + * + * @param string $url Remote URL. + * @param string[] $ips Filled with the validated addresses; stays empty when the host is exempt from the check. + * @return string|WP_Error + */ + private static function validate_url( $url, &$ips = array() ) { + $ips = array(); + $validated_url = wp_http_validate_url( $url ); + if ( false === $validated_url ) { + return new WP_Error( 'visualizer_invalid_remote_url', 'The remote URL is not allowed.' ); + } + + $host = strtolower( rtrim( (string) wp_parse_url( $validated_url, PHP_URL_HOST ), '.' ) ); + + // Mirror core's same-host exemption so media library URLs import on hosts that resolve internally. + $home_host = strtolower( rtrim( (string) wp_parse_url( get_option( 'home' ), PHP_URL_HOST ), '.' ) ); + if ( $host === $home_host ) { + return $validated_url; + } + + $ips = self::resolve_host( $host ); + if ( empty( $ips ) ) { + return new WP_Error( 'visualizer_remote_dns', 'The remote host could not be resolved.' ); + } + + foreach ( $ips as $ip ) { + if ( ! self::is_global_ip( $ip ) ) { + return new WP_Error( 'visualizer_unsafe_remote_url', 'The remote URL resolves to a non-public address.' ); + } + } + + return $validated_url; + } + + /** + * Resolves every IPv4 and IPv6 address for a host. + * + * @param string $host Host name or IP literal. + * @return string[] + */ + private static function resolve_host( $host ) { + if ( filter_var( $host, FILTER_VALIDATE_IP ) ) { + return array( $host ); + } + + $ips = array(); + if ( function_exists( 'dns_get_record' ) ) { + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- DNS failures are handled below. + $records = @dns_get_record( $host, DNS_A | DNS_AAAA ); + foreach ( is_array( $records ) ? $records : array() as $record ) { + if ( ! empty( $record['ip'] ) ) { + $ips[] = $record['ip']; + } elseif ( ! empty( $record['ipv6'] ) ) { + $ips[] = $record['ipv6']; + } + } + } + + if ( empty( $ips ) ) { + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- DNS failures are handled by returning no addresses. + $ipv4 = @gethostbynamel( $host ); + $ips = is_array( $ipv4 ) ? $ipv4 : array(); + } + + return array_values( array_unique( $ips ) ); + } + + /** + * Whether an address is globally routable. + * + * @param string $ip IP address. + * @return bool + */ + private static function is_global_ip( $ip ) { + if ( self::ip_in_range( $ip, '::ffff:0:0/96' ) ) { + $packed = inet_pton( $ip ); + $ip = inet_ntop( substr( $packed, 12 ) ); + } + + if ( false === filter_var( $ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE ) ) { + return false; + } + + $ranges = false !== strpos( $ip, ':' ) + ? array( 'fc00::/7', 'fe80::/10', 'ff00::/8' ) + : array( '100.64.0.0/10', '192.0.0.0/24', '192.0.2.0/24', '198.18.0.0/15', '198.51.100.0/24', '203.0.113.0/24', '224.0.0.0/4' ); + + foreach ( $ranges as $range ) { + if ( self::ip_in_range( $ip, $range ) ) { + return false; + } + } + + return true; + } + + /** + * Checks whether an IP belongs to a CIDR range. + * + * @param string $ip IP address. + * @param string $cidr CIDR range. + * @return bool + */ + private static function ip_in_range( $ip, $cidr ) { + list( $network, $prefix ) = explode( '/', $cidr, 2 ); + $address = inet_pton( $ip ); + $network = inet_pton( $network ); + if ( false === $address || false === $network || strlen( $address ) !== strlen( $network ) ) { + return false; + } + + $bytes = intdiv( (int) $prefix, 8 ); + $bits = (int) $prefix % 8; + if ( substr( $address, 0, $bytes ) !== substr( $network, 0, $bytes ) ) { + return false; + } + + return 0 === $bits || ( ord( $address[ $bytes ] ) & ( 0xff << ( 8 - $bits ) ) ) === ( ord( $network[ $bytes ] ) & ( 0xff << ( 8 - $bits ) ) ); + } + + /** + * Whether two URLs share scheme, host, and port. + * + * @param string $first First URL. + * @param string $second Second URL. + * @return bool + */ + private static function same_origin( $first, $second ) { + $first = wp_parse_url( $first ); + $second = wp_parse_url( $second ); + if ( ! is_array( $first ) || ! is_array( $second ) || empty( $first['scheme'] ) || empty( $first['host'] ) || empty( $second['scheme'] ) || empty( $second['host'] ) ) { + return false; + } + + return strtolower( $first['scheme'] ) === strtolower( $second['scheme'] ) + && strtolower( $first['host'] ) === strtolower( $second['host'] ) + && self::url_port( $first ) === self::url_port( $second ); + } + + /** + * Gets an explicit or scheme-default URL port. + * + * @param array $url Parsed URL. + * @return int + */ + private static function url_port( $url ) { + return isset( $url['port'] ) ? (int) $url['port'] : ( 'https' === strtolower( $url['scheme'] ) ? 443 : 80 ); + } + + /** + * Retains only non-sensitive headers across an origin change. + * + * @param array $headers Request headers. + * @return array + */ + private static function headers_for_cross_origin_redirect( $headers ) { + $allowed = array( 'accept', 'accept-encoding', 'range', 'user-agent' ); + return array_filter( + $headers, + function ( $value, $name ) use ( $allowed ) { + return in_array( strtolower( (string) $name ), $allowed, true ); + }, + ARRAY_FILTER_USE_BOTH + ); + } +} diff --git a/classes/Visualizer/Source/Csv.php b/classes/Visualizer/Source/Csv.php index 1a39e5fc..679714e5 100644 --- a/classes/Visualizer/Source/Csv.php +++ b/classes/Visualizer/Source/Csv.php @@ -117,7 +117,7 @@ private function _fetchSeries( &$handle ) { * * @access protected * @param string $filename Optional file name to get handle. If omitted, $_filename is used. - * @return resource File handle resource on success, otherwise FALSE. + * @return resource|false File handle resource on success, otherwise FALSE. */ protected function _get_file_handle( $filename = false ) { // open file and return handle @@ -141,23 +141,29 @@ public function fetch() { // read file and fill arrays $handle = $this->_get_file_handle(); - if ( $handle ) { - // fetch series - if ( ! $this->_fetchSeries( $handle ) ) { - return false; - } - - // fetch data - $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); - while ( $data !== false ) { - $this->_data[] = $this->_normalizeData( $data ); - $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + if ( ! $handle ) { + if ( empty( $this->_error ) ) { + $this->_error = esc_html__( 'The file could not be opened. Please try again.', 'visualizer' ); } + return false; + } - // close file handle + // fetch series + if ( ! $this->_fetchSeries( $handle ) ) { fclose( $handle ); + return false; } + // fetch data + $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + while ( $data !== false ) { + $this->_data[] = $this->_normalizeData( $data ); + $data = fgetcsv( $handle, 0, VISUALIZER_CSV_DELIMITER, VISUALIZER_CSV_ENCLOSURE ); + } + + // close file handle + fclose( $handle ); + return true; } diff --git a/classes/Visualizer/Source/Csv/Remote.php b/classes/Visualizer/Source/Csv/Remote.php index e034ca15..8eec606f 100644 --- a/classes/Visualizer/Source/Csv/Remote.php +++ b/classes/Visualizer/Source/Csv/Remote.php @@ -30,12 +30,12 @@ class Visualizer_Source_Csv_Remote extends Visualizer_Source_Csv { /** - * Temporary file name used when allow_url_fopen option is disabled. + * Path to the safely downloaded temporary file. * * @since 1.4.2 * * @access private - * @var string + * @var string|false */ private $_tmpfile = false; @@ -129,36 +129,44 @@ public function getSourceName() { return __CLASS__; } + /** + * Fetches the remote file and removes its temporary copy. + * + * @access public + * @return boolean TRUE on success, otherwise FALSE. + */ + public function fetch() { + $result = parent::fetch(); + + if ( $this->_tmpfile && is_file( $this->_tmpfile ) ) { + wp_delete_file( $this->_tmpfile ); + $this->_tmpfile = false; + } + + return $result; + } + /** * Returns file handle to fetch data from. * * @since 1.4.2 * * @access protected - * @staticvar boolean $allow_url_fopen Determines whether or not allow_url_fopen option is enabled. * @param string $filename Optional file name to get handle. If omitted, $_filename is used. - * @return resource File handle resource on success, otherwise FALSE. + * @return resource|false File handle resource on success, otherwise FALSE. */ protected function _get_file_handle( $filename = false ) { - static $allow_url_fopen = null; - - if ( ! is_wp_error( $this->_tmpfile ) && $this->_tmpfile && is_readable( $this->_tmpfile ) ) { + if ( $this->_tmpfile && is_readable( $this->_tmpfile ) ) { return parent::_get_file_handle( $this->_tmpfile ); } - if ( is_null( $allow_url_fopen ) ) { - $allow_url_fopen = filter_var( ini_get( 'allow_url_fopen' ), FILTER_VALIDATE_BOOLEAN ); + $tmpfile = Visualizer_Remote_Fetch::download( $this->_filename ); + if ( is_wp_error( $tmpfile ) ) { + $this->_error = esc_html__( 'Could not download the file. Please check the URL and try again.', 'visualizer' ); + return false; } - $scheme = parse_url( $this->_filename, PHP_URL_SCHEME ); - if ( $allow_url_fopen && in_array( $scheme, stream_get_wrappers(), true ) ) { - return parent::_get_file_handle( $filename ); - } - - require_once ABSPATH . 'wp-admin/includes/file.php'; - - $this->_tmpfile = download_url( $this->_filename ); - - return ! is_wp_error( $this->_tmpfile ) ? parent::_get_file_handle( $this->_tmpfile ) : false; + $this->_tmpfile = $tmpfile; + return parent::_get_file_handle( $this->_tmpfile ); } } diff --git a/classes/Visualizer/Source/Json.php b/classes/Visualizer/Source/Json.php index 5edf29d1..0da21be8 100644 --- a/classes/Visualizer/Source/Json.php +++ b/classes/Visualizer/Source/Json.php @@ -122,7 +122,7 @@ public function __construct( $params = null ) { if ( isset( $this->_args['additional_headers'] ) ) { $this->_additional_headers = $this->_args['additional_headers']; } - do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Constructor called for params = %s', print_r( $params, true ) ), 'debug', __FILE__, __LINE__ ); + do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, 'JSON source initialized.', 'debug', __FILE__, __LINE__ ); } /** @@ -283,7 +283,7 @@ public function fetch() { $this->_data = $data; - do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Parsed data endpoint %s with root %s is %s', $this->_url, $this->_root, print_r( $data, true ) ), 'debug', __FILE__, __LINE__ ); + do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, 'JSON endpoint data parsed.', 'debug', __FILE__, __LINE__ ); return true; } @@ -379,7 +379,7 @@ private function getRootElements( $parent_key, $now, $root, $data ) { } } $roots = array_unique( $root ); - do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Roots found for %s = %s', $this->_url, print_r( $roots, true ) ), 'debug', __FILE__, __LINE__ ); + do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, 'JSON root elements parsed.', 'debug', __FILE__, __LINE__ ); return $roots; } @@ -397,7 +397,8 @@ private function getJSON( $url = null ) { $response = $this->connect( $url ); if ( is_wp_error( $response ) || ! in_array( intval( $response['response']['code'] ), array( 200, 201 ), true ) ) { - do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Error while fetching JSON endpoint %s = %s', $url, print_r( $response, true ) ), 'error', __FILE__, __LINE__ ); + $error_code = is_wp_error( $response ) ? $response->get_error_code() : (string) wp_remote_retrieve_response_code( $response ); + do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Error while fetching JSON endpoint: %s', $error_code ), 'error', __FILE__, __LINE__ ); return null; } @@ -408,7 +409,7 @@ private function getJSON( $url = null ) { $response_body = preg_replace( "/^$bom/", '', $response_body ); $array = apply_filters( 'visualizer_json_massage_data', json_decode( $response_body, true ), $url ); - do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'JSON array for the endpoint is %s = ', print_r( $array, true ) ), 'debug', __FILE__, __LINE__ ); + do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, 'JSON response decoded.', 'debug', __FILE__, __LINE__ ); return $array; } @@ -467,8 +468,8 @@ function ( $headers ) { $args['headers']['X-Visualizer-Token'] = $token; } - do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Connecting to %s with args = %s ', $url, print_r( $args, true ) ), 'debug', __FILE__, __LINE__ ); - return wp_safe_remote_request( $url, $args ); + do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Connecting to JSON endpoint with method %s', $args['method'] ), 'debug', __FILE__, __LINE__ ); + return Visualizer_Remote_Fetch::request( $url, $args ); } /** diff --git a/classes/Visualizer/Source/Xlsx.php b/classes/Visualizer/Source/Xlsx.php index f295209c..f2cf42cd 100644 --- a/classes/Visualizer/Source/Xlsx.php +++ b/classes/Visualizer/Source/Xlsx.php @@ -55,8 +55,13 @@ public function fetch() { } $reader = \OpenSpout\Reader\Common\Creator\ReaderEntityFactory::createXLSXReader(); + $file_path = $this->_get_file_path(); + if ( ! $file_path ) { + return false; + } + try { - $reader->open( $this->_get_file_path() ); + $reader->open( $file_path ); $all_rows = array(); foreach ( $reader->getSheetIterator() as $sheet ) { diff --git a/classes/Visualizer/Source/Xlsx/Remote.php b/classes/Visualizer/Source/Xlsx/Remote.php index 857c2a98..ab482a2f 100644 --- a/classes/Visualizer/Source/Xlsx/Remote.php +++ b/classes/Visualizer/Source/Xlsx/Remote.php @@ -86,41 +86,30 @@ public function getSourceName() { * guarding against ZIP-bomb and DoS attacks. * * @access protected - * @return string Path to the temporary file, or the original URL if download failed. + * @return string|false Path to the temporary file, or false if download failed. */ protected function _get_file_path() { - if ( $this->_tmpfile && ! is_wp_error( $this->_tmpfile ) && is_readable( $this->_tmpfile ) ) { + if ( $this->_tmpfile && is_readable( $this->_tmpfile ) ) { return $this->_tmpfile; } - require_once ABSPATH . 'wp-admin/includes/file.php'; - - $this->_tmpfile = download_url( $this->_filename ); - - if ( is_wp_error( $this->_tmpfile ) ) { - $this->_error = esc_html__( 'Could not download the XLSX file. Please check the URL and try again.', 'visualizer' ); - $this->_tmpfile = false; - // Return the original URL so the parent's open() call will fail - // gracefully and set an error rather than throwing a PHP error. - return $this->_filename; + $max_bytes = (int) apply_filters( 'visualizer_xlsx_max_filesize', 10 * 1024 * 1024 ); + $tmpfile = Visualizer_Remote_Fetch::download( + $this->_filename, + array( 'limit_response_size' => $max_bytes ) + ); + if ( is_wp_error( $tmpfile ) ) { + $this->_error = 'visualizer_remote_size' === $tmpfile->get_error_code() + ? esc_html__( 'The XLSX file exceeds the maximum allowed size and cannot be imported.', 'visualizer' ) + : esc_html__( 'Could not download the XLSX file. Please check the URL and try again.', 'visualizer' ); + return false; } + $this->_tmpfile = $tmpfile; if ( ! is_file( $this->_tmpfile ) ) { $this->_tmpfile = false; $this->_error = esc_html__( 'Could not access the downloaded XLSX file. Please try again.', 'visualizer' ); - return $this->_filename; - } - - // Maximum allowed file size in bytes. Default 10 MB; override via filter. - $max_bytes = (int) apply_filters( 'visualizer_xlsx_max_filesize', 10 * 1024 * 1024 ); - if ( filesize( $this->_tmpfile ) > $max_bytes ) { - @unlink( $this->_tmpfile ); // phpcs:ignore WordPress.PHP.NoSilencedErrors - $this->_tmpfile = false; - $this->_error = esc_html__( - 'The XLSX file exceeds the maximum allowed size and cannot be imported.', - 'visualizer' - ); - return $this->_filename; + return false; } return $this->_tmpfile; @@ -136,8 +125,8 @@ public function fetch() { $result = parent::fetch(); // Clean up the temporary file after parsing. - if ( $this->_tmpfile && ! is_wp_error( $this->_tmpfile ) && is_file( $this->_tmpfile ) ) { - @unlink( $this->_tmpfile ); // phpcs:ignore WordPress.PHP.NoSilencedErrors + if ( $this->_tmpfile && is_file( $this->_tmpfile ) ) { + wp_delete_file( $this->_tmpfile ); $this->_tmpfile = false; } diff --git a/tests/e2e/specs/remote-import.spec.js b/tests/e2e/specs/remote-import.spec.js new file mode 100644 index 00000000..aa459a70 --- /dev/null +++ b/tests/e2e/specs/remote-import.spec.js @@ -0,0 +1,70 @@ +/** + * WordPress dependencies + */ +const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); + +/** + * Internal dependencies + */ +const { deleteAllCharts, getAssetFilePath, CHART_JS_LABELS, selectChartAdmin } = require('../utils/common'); + +/** + * The remote-import UI is Pro-gated, but the admin-ajax endpoint runs in the free + * plugin, so these tests drive the endpoint directly (issue #591 SSRF fix). + * + * On error the endpoint responds with `alert("...")`; on success it assigns + * `win.visualizer.charts.canvas.series` / `.data`. + */ +test.describe( 'Remote import (secure fetch)', () => { + test.beforeEach( async ( { admin, requestUtils, page } ) => { + await deleteAllCharts( requestUtils ); + page.setDefaultTimeout( 5000 ); + } ); + + /** + * Opens the classic builder and returns the upload-data admin-ajax URL + * (carries the nonce and chart id) from the one-time-import form. + */ + async function getUploadAction( admin, page ) { + await admin.visitAdminPage( 'admin.php?page=visualizer&vaction=addnew' ); + await page.waitForURL( '**/admin.php?page=visualizer&vaction=addnew' ); + await page.getByRole('button', { name: 'Classic Builder Step-by-step' }).click(); + await page.waitForSelector('h1:text("Visualizer")'); + + await selectChartAdmin( page.frameLocator('iframe'), CHART_JS_LABELS.table ); + + return page.frameLocator('iframe').locator('#vz-one-time-import').getAttribute('action'); + } + + async function importFromUrl( page, action, url ) { + const response = await page.request.post( action, { + form: { + remote_data: url, + 'vz-import-time': '-1', + }, + } ); + return response.text(); + } + + test( 'blocks import from a non-public address', async ( { admin, page } ) => { + const action = await getUploadAction( admin, page ); + + const body = await importFromUrl( page, action, 'http://169.254.169.254/latest/meta-data/data.csv' ); + + expect( body ).toContain( 'alert(' ); + expect( body ).not.toContain( 'canvas.series' ); + } ); + + test( 'imports a CSV served from the site itself', async ( { admin, page, requestUtils } ) => { + const media = await requestUtils.uploadMedia( getAssetFilePath( 'pie.csv' ) ); + const action = await getUploadAction( admin, page ); + + // Inside the wp-env container the mapped port is unreachable; Apache serves the + // same site on port 80, so PHP can only fetch the file through that. + const containerUrl = media.source_url.replace( /:\d+\//, '/' ); + const body = await importFromUrl( page, action, containerUrl ); + + expect( body ).not.toContain( 'alert(' ); + expect( body ).toContain( 'canvas.series' ); + } ); +} ); diff --git a/tests/test-ajax.php b/tests/test-ajax.php index 20b1a26f..55bac2d0 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -382,39 +382,102 @@ public function test_json_get_data_denied_for_subscriber() { } /** - * A permitted user (contributor) is not blocked by the guard, and the JSON source fetches through the - * SSRF-safe transport (`reject_unsafe_urls`), matching the CSV path (issue #591 SSRF fix). + * JSON get-data admits a user who can edit the chart (issue #591 access-control fix). */ - public function test_json_get_roots_allowed_for_contributor_uses_safe_transport() { - wp_set_current_user( $this->contibutor_user_id ); + public function test_json_get_data_allows_editor_for_editable_chart() { + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_author' => $this->admin_user_id, + ) + ); + $this->_setRole( 'editor' ); + + $filter = function () { + return array( + 'headers' => array(), + 'body' => wp_json_encode( array( array( 'name' => 'a', 'value' => 1 ), array( 'name' => 'b', 'value' => 2 ) ) ), + 'response' => array( 'code' => 200, 'message' => '' ), + 'cookies' => array(), + 'filename' => null, + ); + }; + add_filter( 'pre_http_request', $filter ); + + $_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_DATA . Visualizer_Plugin::VERSION ); + $_POST['params'] = array( + 'url' => 'http://93.184.216.34/data.json', + 'method' => 'GET', + 'chart' => $chart_id, + 'root' => 'root', + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_DATA ); + } catch ( WPAjaxDieContinueException $e ) { + // We expected this, do nothing. + } + remove_filter( 'pre_http_request', $filter ); + + $response = json_decode( $this->_last_response ); + $this->assertIsObject( $response ); + $this->assertTrue( $response->success ); + } + + /** + * JSON get-data dies for a chart the user cannot edit (issue #591 access-control fix). + */ + public function test_json_get_data_denied_for_chart_user_cannot_edit() { + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_author' => $this->admin_user_id, + ) + ); $this->_setRole( 'contributor' ); - $captured = array(); - add_filter( - 'pre_http_request', - function ( $pre, $args, $url ) use ( &$captured ) { - $captured[] = array( - 'url' => $url, - 'reject' => ! empty( $args['reject_unsafe_urls'] ), - ); - return array( - 'headers' => array(), - 'body' => wp_json_encode( array( 'results' => array( array( 'id' => 1 ) ) ) ), - 'response' => array( - 'code' => 200, - 'message' => 'OK', - ), - 'cookies' => array(), - 'filename' => null, - ); - }, - 10, - 3 + $requests = 0; + $filter = function ( $preempt ) use ( &$requests ) { + $requests++; + return $preempt; + }; + add_filter( 'pre_http_request', $filter ); + + $_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_DATA . Visualizer_Plugin::VERSION ); + $_POST['params'] = array( + 'url' => 'http://93.184.216.34/data.json', + 'method' => 'GET', + 'chart' => $chart_id, ); + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_DATA ); + $this->fail( 'Expected the request to die for a chart the user cannot edit.' ); + } catch ( WPAjaxDieStopException $e ) { + $this->assertSame( '', $e->getMessage() ); + } + remove_filter( 'pre_http_request', $filter ); + + $this->assertSame( 0, $requests ); + } + + /** + * A contributor may use JSON import, but link-local destinations are blocked before transport. + */ + public function test_json_get_roots_blocks_link_local_for_contributor() { + wp_set_current_user( $this->contibutor_user_id ); + $this->_setRole( 'contributor' ); + + $requests = 0; + $filter = function ( $preempt ) use ( &$requests ) { + $requests++; + return $preempt; + }; + add_filter( 'pre_http_request', $filter ); + $_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_ROOTS . Visualizer_Plugin::VERSION ); $_POST['params'] = array( - 'url' => 'http://127.0.0.1:9999/latest/meta-data/', + 'url' => 'http://169.254.169.254/latest/meta-data/', 'method' => 'GET', ); @@ -423,17 +486,12 @@ function ( $pre, $args, $url ) use ( &$captured ) { } catch ( WPAjaxDieContinueException $e ) { // We expected this, do nothing. } + remove_filter( 'pre_http_request', $filter ); $response = json_decode( $this->_last_response ); $this->assertIsObject( $response ); - // The capability guard must NOT block a user who has edit_posts. - $msg = isset( $response->data->msg ) ? $response->data->msg : ''; - $this->assertNotEquals( 'You do not have permission to perform this action.', $msg ); - // Every outbound request for the JSON source must use the SSRF-safe transport. - $this->assertNotEmpty( $captured, 'The JSON source did not attempt any fetch.' ); - foreach ( $captured as $req ) { - $this->assertTrue( $req['reject'], 'JSON fetch must use wp_safe_remote_* (reject_unsafe_urls => true).' ); - } + $this->assertFalse( $response->success ); + $this->assertSame( 0, $requests ); } /** diff --git a/tests/test-remote-fetch.php b/tests/test-remote-fetch.php new file mode 100644 index 00000000..8b6027b4 --- /dev/null +++ b/tests/test-remote-fetch.php @@ -0,0 +1,489 @@ +assertWPError( $response ); + $this->assertSame( 0, $requests ); + } + + /** + * Non-public URL examples. + * + * @return array[] + */ + public function blocked_urls() { + return array( + 'loopback' => array( 'http://127.0.0.1/' ), + 'private' => array( 'http://10.0.0.1/' ), + 'link-local' => array( 'http://169.254.169.254/latest/meta-data/' ), + 'carrier-grade' => array( 'http://100.64.0.1/' ), + 'documentation' => array( 'http://192.0.2.1/' ), + 'multicast' => array( 'http://224.0.0.1/' ), + 'reserved' => array( 'http://240.0.0.1/' ), + 'blocked-port' => array( 'http://93.184.216.34:8443/' ), + 'ipv4-mapped' => array( 'http://[::ffff:169.254.169.254]/' ), + ); + } + + /** + * A validated public destination reaches the WordPress HTTP transport. + */ + public function test_allows_public_destination() { + $filter = function ( $preempt, $args, $url ) { + $this->assertSame( 'http://93.184.216.34/data.json', $url ); + $this->assertTrue( $args['reject_unsafe_urls'] ); + return $this->response( 200, array(), '{"ok":true}' ); + }; + add_filter( 'pre_http_request', $filter, 10, 3 ); + + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/data.json' ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertSame( '{"ok":true}', wp_remote_retrieve_body( $response ) ); + } + + /** + * Every redirect target must pass the same destination policy. + */ + public function test_blocks_redirect_to_link_local_destination() { + $requests = 0; + $filter = function () use ( &$requests ) { + $requests++; + return $this->response( 302, array( 'location' => 'http://169.254.169.254/latest/meta-data/' ) ); + }; + add_filter( 'pre_http_request', $filter ); + + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/start' ); + + remove_filter( 'pre_http_request', $filter ); + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_unsafe_remote_url', $response->get_error_code() ); + $this->assertSame( 1, $requests ); + } + + /** + * Sensitive headers must not follow a redirect to another origin. + */ + public function test_strips_sensitive_headers_on_cross_origin_redirect() { + $requests = array(); + $filter = function ( $preempt, $args, $url ) use ( &$requests ) { + $requests[] = array( $url, $args['headers'] ); + if ( 1 === count( $requests ) ) { + return $this->response( 302, array( 'location' => 'http://93.184.216.35/data' ) ); + } + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 3 ); + + $response = Visualizer_Remote_Fetch::request( + 'http://93.184.216.34/start', + array( + 'headers' => array( + 'Accept' => 'application/json', + 'Authorization' => 'Bearer secret', + 'X-Api-Key' => 'secret', + ), + ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertCount( 2, $requests ); + $this->assertSame( array( 'Accept' => 'application/json' ), $requests[1][1] ); + } + + /** + * Cookies must not follow a redirect to another origin. + */ + public function test_drops_cookies_on_cross_origin_redirect() { + $requests = array(); + $filter = function ( $preempt, $args, $url ) use ( &$requests ) { + $requests[] = array( $url, $args['cookies'] ); + if ( 1 === count( $requests ) ) { + return $this->response( 302, array( 'location' => 'http://93.184.216.35/data' ) ); + } + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 3 ); + + $response = Visualizer_Remote_Fetch::request( + 'http://93.184.216.34/start', + array( 'cookies' => array( 'session' => 'secret' ) ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertCount( 2, $requests ); + $this->assertNotEmpty( $requests[0][1] ); + $this->assertSame( array(), $requests[1][1] ); + } + + /** + * The validated addresses are pinned on the cURL transport only while the request dispatches. + */ + public function test_pins_validated_addresses_during_dispatch() { + $pinned_during = null; + $filter = function () use ( &$pinned_during ) { + $pinned_during = has_action( 'http_api_curl' ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter ); + + $response = Visualizer_Remote_Fetch::request( 'http://example.com/data.json' ); + + remove_filter( 'pre_http_request', $filter ); + $this->assertNotWPError( $response ); + $this->assertTrue( $pinned_during ); + $this->assertFalse( has_action( 'http_api_curl' ) ); + } + + /** + * IPv6 addresses use cURL's bracketed CURLOPT_RESOLVE syntax. + */ + public function test_formats_ipv6_addresses_for_curl_resolve() { + $method = new ReflectionMethod( Visualizer_Remote_Fetch::class, 'pin_validated_addresses' ); + $method->setAccessible( true ); + $pin = $method->invoke( + null, + 'https://example.com/data.json', + array( '93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946' ) + ); + + $this->assertIsCallable( $pin ); + $reflection = new ReflectionFunction( $pin ); + $this->assertSame( + 'example.com:443:93.184.216.34,[2606:2800:220:1:248:1893:25c8:1946]', + $reflection->getStaticVariables()['entry'] + ); + remove_action( 'http_api_curl', $pin ); + } + + /** + * Header and method policy is applied before dispatch. + */ + public function test_rejects_unsupported_method_before_request() { + $requests = 0; + $filter = function ( $preempt ) use ( &$requests ) { + $requests++; + return $preempt; + }; + add_filter( 'pre_http_request', $filter ); + + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/', array( 'method' => 'DELETE' ) ); + + remove_filter( 'pre_http_request', $filter ); + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_remote_method', $response->get_error_code() ); + $this->assertSame( 0, $requests ); + } + + /** + * URLs on the site's own host skip the public-address check, matching core. + */ + public function test_allows_same_host_destination_without_dns_check() { + // A filter beats the WP_HOME constant pinned by some test configs (e.g. wp-env), which makes update_option() a no-op. + add_filter( + 'option_home', + function () { + return 'http://visualizer.internal'; + }, + 100 + ); + + $requests = 0; + $filter = function ( $preempt ) use ( &$requests ) { + $requests++; + return $this->response( 200, array(), 'a,b' ); + }; + add_filter( 'pre_http_request', $filter ); + + $response = Visualizer_Remote_Fetch::request( 'http://visualizer.internal/wp-content/uploads/data.csv' ); + + remove_filter( 'pre_http_request', $filter ); + $this->assertNotWPError( $response ); + $this->assertSame( 1, $requests ); + } + + /** + * A same-origin redirect (relative Location) keeps request headers and resolves the target URL. + */ + public function test_same_origin_redirect_keeps_headers_and_resolves_relative_location() { + $requests = array(); + $filter = function ( $preempt, $args, $url ) use ( &$requests ) { + $requests[] = array( $url, $args['headers'] ); + if ( 1 === count( $requests ) ) { + return $this->response( 302, array( 'location' => '/next' ) ); + } + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 3 ); + + $headers = array( + 'Accept' => 'application/json', + 'Authorization' => 'Bearer secret', + ); + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/start', array( 'headers' => $headers ) ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertCount( 2, $requests ); + $this->assertSame( 'http://93.184.216.34/next', $requests[1][0] ); + $this->assertSame( $headers, $requests[1][1] ); + } + + /** + * The redirect chain must stop after MAX_REDIRECTS hops. + */ + public function test_stops_after_max_redirects() { + $requests = 0; + $filter = function () use ( &$requests ) { + $requests++; + return $this->response( 302, array( 'location' => 'http://93.184.216.34/hop' . $requests ) ); + }; + add_filter( 'pre_http_request', $filter ); + + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/start' ); + + remove_filter( 'pre_http_request', $filter ); + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_too_many_redirects', $response->get_error_code() ); + $this->assertSame( Visualizer_Remote_Fetch::MAX_REDIRECTS + 1, $requests ); + } + + /** + * A successful download streams to a temporary file and returns its path. + */ + public function test_download_returns_temp_file_with_body() { + $filter = function ( $preempt, $args ) { + $this->assertTrue( $args['stream'] ); + $this->assertNotEmpty( $args['filename'] ); + file_put_contents( $args['filename'], 'col1,col2' ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $tmpfile = Visualizer_Remote_Fetch::download( 'http://93.184.216.34/data.csv' ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $tmpfile ); + $this->assertSame( 'col1,col2', file_get_contents( $tmpfile ) ); + wp_delete_file( $tmpfile ); + } + + /** + * A failed download must not leave the temporary file behind. + */ + public function test_download_removes_temp_file_on_error_status() { + $tmpfile = null; + $filter = function ( $preempt, $args ) use ( &$tmpfile ) { + $tmpfile = $args['filename']; + file_put_contents( $tmpfile, 'not found' ); + return $this->response( 404 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::download( 'http://93.184.216.34/data.csv' ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_remote_status', $response->get_error_code() ); + $this->assertNotEmpty( $tmpfile ); + $this->assertFileDoesNotExist( $tmpfile ); + } + + /** + * Downloads are size-capped during transfer and possibly-truncated files are rejected. + */ + public function test_download_rejects_file_over_size_limit() { + $tmpfile = null; + $filter = function ( $preempt, $args ) use ( &$tmpfile ) { + $this->assertSame( Visualizer_Remote_Fetch::MAX_DOWNLOAD_BYTES + 1, $args['limit_response_size'] ); + $tmpfile = $args['filename']; + file_put_contents( $tmpfile, str_repeat( 'a', Visualizer_Remote_Fetch::MAX_DOWNLOAD_BYTES + 1 ) ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::download( 'http://93.184.216.34/data.csv' ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_remote_size', $response->get_error_code() ); + $this->assertNotEmpty( $tmpfile ); + $this->assertFileDoesNotExist( $tmpfile ); + } + + /** + * A complete file exactly at the configured limit remains valid. + */ + public function test_download_accepts_file_at_size_limit() { + $filter = function ( $preempt, $args ) { + $this->assertSame( 5, $args['limit_response_size'] ); + file_put_contents( $args['filename'], '1234' ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $tmpfile = Visualizer_Remote_Fetch::download( + 'http://93.184.216.34/data.csv', + array( 'limit_response_size' => 4 ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $tmpfile ); + $this->assertSame( '1234', file_get_contents( $tmpfile ) ); + wp_delete_file( $tmpfile ); + } + + /** + * Callers may provide a smaller or larger download limit. + */ + public function test_download_honors_custom_size_limit() { + $tmpfile = null; + $filter = function ( $preempt, $args ) use ( &$tmpfile ) { + $this->assertSame( 5, $args['limit_response_size'] ); + $tmpfile = $args['filename']; + file_put_contents( $tmpfile, '12345' ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::download( + 'http://93.184.216.34/data.csv', + array( 'limit_response_size' => 4 ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_remote_size', $response->get_error_code() ); + $this->assertNotEmpty( $tmpfile ); + $this->assertFileDoesNotExist( $tmpfile ); + } + + /** + * XLSX probes only need the four-byte ZIP magic number. + * + * @dataProvider xlsx_probe_callbacks + * @param string $class Probe owner. + */ + public function test_xlsx_probes_limit_response_to_magic_bytes( $class ) { + $filter = function ( $preempt, $args ) { + $this->assertSame( 4, $args['limit_response_size'] ); + file_put_contents( $args['filename'], "PK\x03\x04" ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $method = new ReflectionMethod( $class, '_url_is_xlsx' ); + $method->setAccessible( true ); + $result = $method->invoke( null, 'http://93.184.216.34/download' ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertTrue( $result ); + } + + /** + * XLSX probe owners. + * + * @return array[] + */ + public function xlsx_probe_callbacks() { + return array( + 'classic' => array( Visualizer_Module_Chart::class ), + 'ai builder' => array( Visualizer_Module_AIBuilder::class ), + ); + } + + /** + * The XLSX-specific filter controls the gateway download limit. + */ + public function test_xlsx_download_uses_filtered_size_limit() { + $max_bytes = 12 * 1024 * 1024; + $limit = function () use ( $max_bytes ) { + return $max_bytes; + }; + add_filter( 'visualizer_xlsx_max_filesize', $limit ); + + $filter = function ( $preempt, $args ) use ( $max_bytes ) { + $this->assertSame( $max_bytes + 1, $args['limit_response_size'] ); + file_put_contents( $args['filename'], 'xlsx' ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $source = new Visualizer_Source_Xlsx_Remote( 'http://93.184.216.34/data.xlsx' ); + $method = new ReflectionMethod( $source, '_get_file_path' ); + $method->setAccessible( true ); + $path = $method->invoke( $source ); + + remove_filter( 'visualizer_xlsx_max_filesize', $limit ); + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertIsString( $path ); + wp_delete_file( $path ); + } + + /** + * Downloads enforce the same destination policy as plain requests. + */ + public function test_download_blocks_non_public_destination() { + $requests = 0; + $filter = function ( $preempt ) use ( &$requests ) { + $requests++; + return $preempt; + }; + add_filter( 'pre_http_request', $filter ); + + $response = Visualizer_Remote_Fetch::download( 'http://169.254.169.254/latest/meta-data/' ); + + remove_filter( 'pre_http_request', $filter ); + $this->assertWPError( $response ); + $this->assertSame( 0, $requests ); + } + + /** + * Builds a WordPress HTTP response fixture. + * + * @param int $code Status code. + * @param array $headers Response headers. + * @param string $body Response body. + * @return array + */ + private function response( $code, $headers = array(), $body = '' ) { + return array( + 'headers' => $headers, + 'body' => $body, + 'response' => array( + 'code' => $code, + 'message' => '', + ), + 'cookies' => array(), + 'filename' => null, + ); + } +} From 9a7fee5020eb9bfa5ed1180a7c847d384eecd615 Mon Sep 17 00:00:00 2001 From: Soare Robert Daniel Date: Wed, 15 Jul 2026 10:27:47 +0300 Subject: [PATCH 05/12] fix: secure AI Builder data endpoints (#1337) --- classes/Visualizer/Module/AIBuilder.php | 27 ++++--- tests/test-ajax.php | 96 +++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 11 deletions(-) diff --git a/classes/Visualizer/Module/AIBuilder.php b/classes/Visualizer/Module/AIBuilder.php index c01087eb..104c647f 100644 --- a/classes/Visualizer/Module/AIBuilder.php +++ b/classes/Visualizer/Module/AIBuilder.php @@ -94,6 +94,17 @@ private function _verify_create_nonce(): void { } } + /** + * Verify that the current user can edit a chart. + * + * @param int $chart_id Chart ID. + */ + private function _verify_chart_access( $chart_id ): void { + if ( ! current_user_can( 'edit_post', $chart_id ) ) { + wp_send_json_error( array( 'message' => __( 'Unauthorized.', 'visualizer' ) ), 403 ); + } + } + /** * Persist chart data + series from a source. * @@ -159,6 +170,7 @@ public function getChartNonce(): void { if ( ! $chart_id || ! get_post( $chart_id ) ) { wp_send_json_error( array( 'message' => __( 'Chart not found.', 'visualizer' ) ) ); } + $this->_verify_chart_access( $chart_id ); wp_send_json_success( array( 'upload_nonce' => wp_create_nonce( 'visualizer-ai-upload-' . $chart_id ), @@ -180,6 +192,7 @@ public function fetchChart(): void { if ( ! $chart || $chart->post_type !== Visualizer_Plugin::CPT_VISUALIZER ) { wp_send_json_error( array( 'message' => __( 'Chart not found.', 'visualizer' ) ) ); } + $this->_verify_chart_access( $chart_id ); $series = get_post_meta( $chart_id, Visualizer_Plugin::CF_SERIES, true ); $data = Visualizer_Module::get_chart_data( $chart, '', false ); @@ -259,6 +272,7 @@ public function uploadData(): void { if ( ! get_post( $chart_id ) ) { wp_send_json_error( array( 'message' => __( 'Chart not found.', 'visualizer' ) ) ); } + $this->_verify_chart_access( $chart_id ); $source_type = isset( $_POST['source_type'] ) ? sanitize_key( $_POST['source_type'] ) : 'csv_string'; $source = null; @@ -298,17 +312,6 @@ public function uploadData(): void { } $url = wp_unslash( $_POST['file_url'] ); - // Allow local absolute paths in dev (same CSVs used by Classic). - if ( is_string( $url ) && file_exists( $url ) && is_readable( $url ) ) { - $ext = strtolower( pathinfo( $url, PATHINFO_EXTENSION ) ); - if ( 'xlsx' === $ext && class_exists( 'Visualizer_Source_Xlsx' ) ) { - $source = new Visualizer_Source_Xlsx( $url ); - } else { - $source = new Visualizer_Source_Csv( $url ); - } - break; - } - if ( function_exists( 'wp_http_validate_url' ) ) { $validated_url = wp_http_validate_url( (string) $url ); $url = false === $validated_url ? false : (string) $validated_url; @@ -436,6 +439,7 @@ public function generateChart(): void { if ( ! $chart_id || ! get_post( $chart_id ) ) { wp_send_json_error( array( 'message' => __( 'Chart not found.', 'visualizer' ) ) ); } + $this->_verify_chart_access( $chart_id ); $prompt = isset( $_POST['prompt'] ) ? sanitize_textarea_field( wp_unslash( $_POST['prompt'] ) ) : ''; $series = isset( $_POST['series'] ) ? wp_unslash( $_POST['series'] ) : ''; @@ -561,6 +565,7 @@ public function saveChart(): void { if ( ! $chart_id || ! get_post( $chart_id ) ) { wp_send_json_error( array( 'message' => __( 'Chart not found.', 'visualizer' ) ) ); } + $this->_verify_chart_access( $chart_id ); if ( empty( $code ) ) { wp_send_json_error( array( 'message' => __( 'No chart code found. Generate a chart first.', 'visualizer' ) ) ); } diff --git a/tests/test-ajax.php b/tests/test-ajax.php index 55bac2d0..a003e038 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -60,6 +60,102 @@ public function setUp(): void { } + /** + * Test that the AI Builder URL import rejects local file paths. + */ + public function test_ai_builder_file_url_rejects_local_path() { + wp_set_current_user( $this->contibutor_user_id ); + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'draft', + 'post_author' => $this->contibutor_user_id, + ) + ); + $file = wp_tempnam( 'visualizer-local.csv' ); + file_put_contents( $file, "Label,Value\nstring,number\nSecret,42" ); + + $_POST = array( + 'chart_id' => $chart_id, + 'nonce' => wp_create_nonce( 'visualizer-ai-upload-' . $chart_id ), + 'source_type' => 'file_url', + 'file_url' => $file, + ); + + try { + $this->_handleAjax( 'visualizer-ai-upload' ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after wp_send_json_error(). + } + wp_delete_file( $file ); + + $response = json_decode( $this->_last_response ); + $this->assertFalse( $response->success ); + $this->assertSame( 'Invalid URL. Please check the URL and try again.', $response->data->message ); + } + + /** + * Test that a user cannot request an upload nonce for another user's chart. + */ + public function test_ai_builder_chart_nonce_requires_chart_edit_permission() { + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'publish', + 'post_author' => $this->admin_user_id, + ) + ); + wp_set_current_user( $this->contibutor_user_id ); + + $_POST = array( + 'chart_id' => $chart_id, + 'nonce' => wp_create_nonce( 'visualizer-ai-builder' ), + ); + + try { + $this->_handleAjax( 'visualizer-ai-chart-nonce' ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after wp_send_json_error(). + } + + $response = json_decode( $this->_last_response ); + $this->assertFalse( $response->success ); + $this->assertSame( 'Unauthorized.', $response->data->message ); + } + + /** + * Test that a user cannot upload data to another user's chart. + */ + public function test_ai_builder_upload_requires_chart_edit_permission() { + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'publish', + 'post_author' => $this->admin_user_id, + ) + ); + $original_content = get_post_field( 'post_content', $chart_id ); + wp_set_current_user( $this->contibutor_user_id ); + + $_POST = array( + 'chart_id' => $chart_id, + 'nonce' => wp_create_nonce( 'visualizer-ai-upload-' . $chart_id ), + 'source_type' => 'csv_string', + 'csv_data' => "Label,Value\nstring,number\nSecret,42", + ); + + try { + $this->_handleAjax( 'visualizer-ai-upload' ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after wp_send_json_error(). + } + + $response = json_decode( $this->_last_response ); + $this->assertFalse( $response->success ); + $this->assertSame( 'Unauthorized.', $response->data->message ); + $this->assertSame( $original_content, get_post_field( 'post_content', $chart_id ) ); + } + /** * Test the AJAX response for fetching the database data. */ From 8699a77aea8122623f958af068a9a3900d5a13f4 Mon Sep 17 00:00:00 2001 From: Girish Panchal <79647963+girishpanchal30@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:06:57 +0530 Subject: [PATCH 06/12] fix: sanitize data to prevent XSS for $post_settings (#1338) --- classes/Visualizer/Module/Chart.php | 26 ++++++++ classes/Visualizer/Render/Library.php | 4 +- classes/Visualizer/Render/Page/Data.php | 2 +- tests/test-sanitize-settings.php | 88 +++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 tests/test-sanitize-settings.php diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index 0fffdb5b..58262d5a 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -824,6 +824,7 @@ private function _handleDataAndSettingsPage() { if ( isset( $existing['colors'] ) && is_array( $existing['colors'] ) && ! isset( $post_settings['colors'] ) ) { $post_settings['colors'] = $existing['colors']; } + $post_settings = $this->sanitizeSettings( $post_settings ); update_post_meta( $this->_chart->ID, Visualizer_Plugin::CF_SETTINGS, $post_settings ); // we will keep a parameter called 'internal_title' that will be set to the given title or, if empty, the chart ID @@ -1015,6 +1016,31 @@ private function _handleTypesPage() { wp_iframe( array( $render, 'render' ) ); } + /** + * Sanitize settings data from the request. + * + * @param array $post_data The POST data to sanitize. + * @return array The sanitized settings data. + */ + private function sanitizeSettings( $post_data ): array { + $chart_img = ''; + if ( isset( $post_data['chart-img'] ) ) { + $chart_img = wp_unslash( $post_data['chart-img'] ); + unset( $post_data['chart-img'] ); + } + + $post_data = map_deep( + $post_data, + 'sanitize_textarea_field' + ); + + if ( '' !== $chart_img ) { + $post_data['chart-img'] = $chart_img; + } + + return $post_data; + } + /** * Renders flattr script in the iframe * diff --git a/classes/Visualizer/Render/Library.php b/classes/Visualizer/Render/Library.php index f0e6ba21..d326fe4c 100644 --- a/classes/Visualizer/Render/Library.php +++ b/classes/Visualizer/Render/Library.php @@ -496,10 +496,10 @@ private function _renderChartBox( $placeholder_id, $chart_id, $with_filter = fal } echo '
'; echo '
'; - echo '' . esc_html__( 'Edit', 'visualizer' ) . '' . esc_html__( 'Edit', 'visualizer' ) . ''; + echo '' . esc_html__( 'Edit', 'visualizer' ) . '' . esc_html__( 'Edit', 'visualizer' ) . ''; echo '' . esc_html__( 'Copy Shortcode', 'visualizer' ) . ''; if ( $this->can_chart_have_action( 'image', $chart_id ) ) { - echo '' . esc_html__( 'Download PNG', 'visualizer' ) . ''; + echo '' . esc_html__( 'Download PNG', 'visualizer' ) . ''; } echo '' . esc_html__( 'Export CSV', 'visualizer' ) . ''; echo '' . esc_html__( 'Duplicate', 'visualizer' ) . ''; diff --git a/classes/Visualizer/Render/Page/Data.php b/classes/Visualizer/Render/Page/Data.php index 9ad6b236..58543fd4 100644 --- a/classes/Visualizer/Render/Page/Data.php +++ b/classes/Visualizer/Render/Page/Data.php @@ -93,7 +93,7 @@ protected function _renderSidebarContent() {
- +
diff --git a/tests/test-sanitize-settings.php b/tests/test-sanitize-settings.php new file mode 100644 index 00000000..20d24cdf --- /dev/null +++ b/tests/test-sanitize-settings.php @@ -0,0 +1,88 @@ +setAccessible( true ); + return $method->invoke( $module, $post_data ); + } + + /** + * Script tags in settings values are stripped. + */ + public function test_strips_script_tags_from_values() { + $result = $this->sanitize( + array( + 'backend-title' => 'My Chart', + ) + ); + $this->assertSame( 'My Chart', $result['backend-title'] ); + } + + /** + * Sanitization is applied to nested arrays. + */ + public function test_sanitizes_nested_values() { + $result = $this->sanitize( + array( + 'title' => array( + 'text' => 'Sales', + ), + 'series' => array( + array( 'label' => 'Q1' ), + ), + ) + ); + $this->assertSame( 'Sales', $result['title']['text'] ); + $this->assertSame( 'Q1', $result['series'][0]['label'] ); + } + + /** + * The chart-img value (a data URI) must pass through unsanitized. + */ + public function test_chart_img_is_preserved() { + $img = 'data:image/png;base64,iVBORw0KGgo='; + $result = $this->sanitize( + array( + 'chart-img' => $img, + 'backend-title' => 'Title', + ) + ); + $this->assertSame( $img, $result['chart-img'] ); + $this->assertSame( 'Title', $result['backend-title'] ); + } + + /** + * An empty chart-img is dropped, not re-added. + */ + public function test_empty_chart_img_is_dropped() { + $result = $this->sanitize( array( 'chart-img' => '' ) ); + $this->assertArrayNotHasKey( 'chart-img', $result ); + } + + /** + * Multi-line values keep their newlines (textarea sanitization, not text). + */ + public function test_newlines_are_preserved() { + $result = $this->sanitize( array( 'description' => "Line one\nLine two" ) ); + $this->assertSame( "Line one\nLine two", $result['description'] ); + } +} From d25906bf0451431eb60816ef0f68d359453c82a8 Mon Sep 17 00:00:00 2001 From: Soare Robert Daniel Date: Wed, 15 Jul 2026 12:26:23 +0300 Subject: [PATCH 07/12] test: cover settings sanitization in the chart save flow (#1341) --- tests/test-ajax.php | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/test-ajax.php b/tests/test-ajax.php index a003e038..ac9c9975 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -94,6 +94,51 @@ public function test_ai_builder_file_url_rejects_local_path() { $this->assertSame( 'Invalid URL. Please check the URL and try again.', $response->data->message ); } + /** + * Test that saving chart settings sanitizes the stored settings meta. + */ + public function test_edit_chart_save_sanitizes_settings_meta() { + wp_set_current_user( $this->admin_user_id ); + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'publish', + 'post_author' => $this->admin_user_id, + ) + ); + add_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'line' ); + + $original_request_method = isset( $_SERVER['REQUEST_METHOD'] ) ? $_SERVER['REQUEST_METHOD'] : null; + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_GET = array( + 'chart' => $chart_id, + 'tab' => 'settings', + 'nonce' => wp_create_nonce(), + ); + // No literal '<' so the payload survives the wp_strip_all_tags() pass at + // the top of renderChartPages(); only sanitizeSettings() removes the + // percent-encoded octets. This fails if the sanitizeSettings() call is + // dropped from the save path. + $_POST = array( + 'backend-title' => 'Chart %3Cscript%3E', + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_EDIT_CHART ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected. + } finally { + if ( null === $original_request_method ) { + unset( $_SERVER['REQUEST_METHOD'] ); + } else { + $_SERVER['REQUEST_METHOD'] = $original_request_method; + } + } + + $settings = get_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, true ); + $this->assertSame( 'Chart script', $settings['backend-title'] ); + } + /** * Test that a user cannot request an upload nonce for another user's chart. */ From 09eaf4cc79e0b29f40356a481a75ef24bd3764d2 Mon Sep 17 00:00:00 2001 From: lucadobrescu Date: Wed, 15 Jul 2026 14:38:34 +0300 Subject: [PATCH 08/12] fix: harden unserialize() against PHP object injection (#1339) --- classes/Visualizer/Gutenberg/Block.php | 2 +- classes/Visualizer/Module.php | 52 +++- classes/Visualizer/Module/Admin.php | 4 +- classes/Visualizer/Module/Chart.php | 8 +- classes/Visualizer/Module/Utility.php | 5 +- classes/Visualizer/Module/Wizard.php | 2 +- classes/Visualizer/Source/Csv/Remote.php | 2 +- tests/test-ajax.php | 32 +++ tests/test-security-object-injection.php | 352 +++++++++++++++++++++++ 9 files changed, 447 insertions(+), 12 deletions(-) create mode 100644 tests/test-security-object-injection.php diff --git a/classes/Visualizer/Gutenberg/Block.php b/classes/Visualizer/Gutenberg/Block.php index 61d58fe1..1111c86b 100644 --- a/classes/Visualizer/Gutenberg/Block.php +++ b/classes/Visualizer/Gutenberg/Block.php @@ -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. diff --git a/classes/Visualizer/Module.php b/classes/Visualizer/Module.php index 87add67a..9d8166bb 100644 --- a/classes/Visualizer/Module.php +++ b/classes/Visualizer/Module.php @@ -778,6 +778,56 @@ 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; + } + return self::strip_incomplete_objects( unserialize( trim( $content ), array( 'allowed_classes' => 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. */ @@ -793,7 +843,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 ) { diff --git a/classes/Visualizer/Module/Admin.php b/classes/Visualizer/Module/Admin.php index 748bf9a3..7b9c1e5d 100644 --- a/classes/Visualizer/Module/Admin.php +++ b/classes/Visualizer/Module/Admin.php @@ -223,7 +223,7 @@ public function addRevision( $revision_id ) { if ( $meta ) { foreach ( $meta as $key => $value ) { if ( 0 === strpos( $key, 'visualizer' ) ) { - add_metadata( 'post', $revision_id, $key, maybe_unserialize( $value[0] ) ); + add_metadata( 'post', $revision_id, $key, self::maybe_decode_content( $value[0] ) ); } } } @@ -251,7 +251,7 @@ public function restoreRevision( $post_id, $revision_id ) { if ( $meta ) { foreach ( $meta as $key => $value ) { if ( 0 === strpos( $key, 'visualizer' ) ) { - add_post_meta( $post_id, $key, maybe_unserialize( $value[0] ) ); + add_post_meta( $post_id, $key, self::maybe_decode_content( $value[0] ) ); } } } diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index 58262d5a..65b9fcef 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -614,7 +614,7 @@ public function renderChartPages() { $chart_id = $new_chart_id; foreach ( $post_meta as $key => $value ) { if ( strpos( $key, 'visualizer-' ) !== false ) { - add_post_meta( $new_chart_id, $key, maybe_unserialize( $value[0] ) ); + add_post_meta( $new_chart_id, $key, self::maybe_decode_content( $value[0] ) ); } } } @@ -1364,8 +1364,8 @@ public function uploadData() { if ( $source->fetch() ) { $content = $source->getData( get_post_meta( $chart_id, Visualizer_Plugin::CF_EDITABLE_TABLE, true ) ); $populate = true; - if ( is_string( $content ) && is_array( unserialize( $content ) ) ) { - $json = unserialize( $content ); + $json = self::decode_content( $content ); + if ( is_array( $json ) ) { // if source exists, so should data. if source exists but data is blank, do not populate the chart. // if we populate the data even if it is empty, the chart will show "Table has no columns". if ( array_key_exists( 'source', $json ) && ! empty( $json['source'] ) && ( ! array_key_exists( 'data', $json ) || empty( $json['data'] ) ) ) { @@ -1456,7 +1456,7 @@ public function cloneChart() { $post_meta = get_post_meta( $chart_id ); foreach ( $post_meta as $key => $value ) { if ( strpos( $key, 'visualizer-' ) !== false ) { - add_post_meta( $new_chart_id, $key, maybe_unserialize( $value[0] ) ); + add_post_meta( $new_chart_id, $key, self::maybe_decode_content( $value[0] ) ); } } $redirect = esc_url( diff --git a/classes/Visualizer/Module/Utility.php b/classes/Visualizer/Module/Utility.php index 395ae140..3858190f 100644 --- a/classes/Visualizer/Module/Utility.php +++ b/classes/Visualizer/Module/Utility.php @@ -302,7 +302,8 @@ private static function apply_chartjs_palette( array $settings, string $type, ar // fall through. case 'pie': $chart = get_post( $chart_id ); - $data = $chart instanceof WP_Post ? maybe_unserialize( $chart->post_content ) : array(); + $raw = $chart instanceof WP_Post ? $chart->post_content : array(); + $data = self::maybe_decode_content( $raw ); $name = 'slices'; $max = is_array( $data ) ? count( $data ) : $count; // fall through. @@ -434,7 +435,7 @@ private static function set_defaults_chartjs( $chart, $post_status ) { case 'polarArea': // fall through. case 'pie': - $data = unserialize( $chart->post_content ); + $data = self::decode_content( $chart->post_content ); $name = 'slices'; $max = count( $data ); // fall through. diff --git a/classes/Visualizer/Module/Wizard.php b/classes/Visualizer/Module/Wizard.php index 607500f1..3a100752 100644 --- a/classes/Visualizer/Module/Wizard.php +++ b/classes/Visualizer/Module/Wizard.php @@ -234,7 +234,7 @@ private function setup_wizard_import_chart() { update_post_meta( $chart_id, Visualizer_Plugin::CF_SERIES, $series ); update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_LIBRARY, '' ); - $data = maybe_unserialize( $data ); + $data = Visualizer_Module::decode_content( $data ); $setting_series = array(); $setting_slices = array(); foreach ( $data as $s ) { diff --git a/classes/Visualizer/Source/Csv/Remote.php b/classes/Visualizer/Source/Csv/Remote.php index 8eec606f..1554dd0c 100644 --- a/classes/Visualizer/Source/Csv/Remote.php +++ b/classes/Visualizer/Source/Csv/Remote.php @@ -74,7 +74,7 @@ private function _repopulate( $chart_id ) { // if filename is empty, extract it from chart content if ( empty( $this->_filename ) ) { $chart = get_post( $chart_id ); - $data = unserialize( html_entity_decode( $chart->post_content ) ); + $data = Visualizer_Module::decode_content( html_entity_decode( $chart->post_content ) ); if ( ! isset( $data['source'] ) ) { return false; } diff --git a/tests/test-ajax.php b/tests/test-ajax.php index ac9c9975..392fbc49 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -635,6 +635,38 @@ public function test_json_get_roots_blocks_link_local_for_contributor() { $this->assertSame( 0, $requests ); } + /** + * Test that the setup wizard import step builds per-row settings from the + * decoded sample data, covering the decode_content() call in the wizard. + */ + public function test_wizard_import_chart_builds_settings_from_decoded_sample_data() { + wp_set_current_user( $this->admin_user_id ); + + $_POST = array( + 'security' => wp_create_nonce( VISUALIZER_ABSPATH ), + 'step' => 'step_2', + 'chart_type' => 'pie', + ); + + try { + $this->_handleAjax( 'visualizer_wizard_step_process' ); + } catch ( WPAjaxDieContinueException $e ) { + // We expected this, do nothing. + } + + // Skip any PHP notices emitted before the JSON payload. + $response = json_decode( substr( $this->_last_response, (int) strpos( $this->_last_response, '{' ) ) ); + $this->assertSame( 1, $response->success ); + + // Data rows in the bundled sample = total lines minus label + type rows. + $expected_rows = count( array_filter( array_map( 'trim', file( VISUALIZER_ABSPATH . '/samples/pie.csv' ) ) ) ) - 2; + $settings = get_post_meta( $response->chart_id, Visualizer_Plugin::CF_SETTINGS, true ); + + $this->assertGreaterThan( 0, $expected_rows ); + $this->assertCount( $expected_rows, $settings['series'] ); + $this->assertCount( $expected_rows, $settings['slices'] ); + } + /** * Utility method to mock pro version. */ diff --git a/tests/test-security-object-injection.php b/tests/test-security-object-injection.php new file mode 100644 index 00000000..15fdf9f6 --- /dev/null +++ b/tests/test-security-object-injection.php @@ -0,0 +1,352 @@ + false ) guard, a serialized object in the + * content must NOT be instantiated. The canary below flips a static flag + * from its __wakeup(); if the guard is removed, unserialize() instantiates + * the canary, __wakeup() fires, and the assertion fails. + * + * @package Visualizer + * @subpackage Tests + */ + +if ( ! class_exists( 'Visualizer_POI_Canary' ) ) { + /** + * Canary "gadget": records whether it was ever instantiated by unserialize(). + */ + class Visualizer_POI_Canary { + /** + * Flag to track whether the canary was instantiated. + * + * @var bool + */ + public static $awoke = false; + + /** + * Records that this object was instantiated by unserialize(). + */ + public function __wakeup() { + self::$awoke = true; + } + } +} + +/** + * Security tests for object injection vulnerabilities. + */ +class Test_Security_Object_Injection extends WP_UnitTestCase { + + /** + * Serialized array payload carrying a Visualizer_POI_Canary object. + * + * @param array $data Plain payload data. + * @return string + */ + private function object_payload( $data = array() ) { + Visualizer_POI_Canary::$awoke = false; + $data[] = new Visualizer_POI_Canary(); + return serialize( $data ); + } + + /** + * Visualizer_Module::get_chart_data() must not instantiate objects from content. + */ + public function test_get_chart_data_does_not_instantiate_objects() { + $expected = array( + array( 'Label', 'Value' ), + array( 'Safe', 10 ), + ); + $chart = new stdClass(); + $chart->post_content = $this->object_payload( $expected ); + + $result = Visualizer_Module::get_chart_data( $chart, 'line', false ); + + $this->assertSame( $expected, $result ); + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'unserialize() must not instantiate objects from chart post_content.' + ); + } + + /** + * The remote CSV source (_repopulate) must not instantiate objects from content. + */ + public function test_remote_csv_source_does_not_instantiate_objects() { + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_content' => wp_slash( $this->object_payload( array( 'marker' => 'remote-csv' ) ) ), + ) + ); + + $source = new Visualizer_Source_Csv_Remote(); + $method = new ReflectionMethod( 'Visualizer_Source_Csv_Remote', '_repopulate' ); + $method->setAccessible( true ); + + $result = $method->invoke( $source, $chart_id ); + + $this->assertFalse( $result, 'Content without a remote source must fail cleanly.' ); + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'Remote CSV source must not instantiate objects from chart post_content.' + ); + } + + /** + * The shared decode_content() chokepoint must not instantiate objects. + * + * Covers the helper-level guarantee shared by chart/source content callers. + */ + public function test_decode_content_does_not_instantiate_objects() { + $result = Visualizer_Module::decode_content( $this->object_payload( array( 'marker' => 'decoded' ) ) ); + + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'decode_content() must not instantiate objects from source content.' + ); + $this->assertSame( + array( 'marker' => 'decoded' ), + $result, + 'decode_content() must keep legitimate data and strip object stubs entirely.' + ); + } + + /** + * The Utility pie/polarArea render palette call site must not instantiate objects. + * + * Covers the public global-style filter seam and preserves the trimming + * behavior of WordPress's maybe_unserialize(). + */ + public function test_utility_pie_palette_call_site_is_guarded() { + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_content' => wp_slash( + " \n" . $this->object_payload( + array( + array( 'One' ), + array( 'Two' ), + ) + ) . " \n" + ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_LIBRARY, 'ChartJS' ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'pie' ); + update_post_meta( + $chart_id, + Visualizer_Plugin::CF_SERIES, + array( + array( 'label' => 'Label', 'type' => 'string' ), + array( 'label' => 'Value', 'type' => 'number' ), + ) + ); + update_option( + Visualizer_Module_Admin::OPTION_GLOBAL_SETTINGS, + array( + 'color_primary' => '#3366cc', + 'apply_existing' => '1', + ) + ); + + $utility = Visualizer_Plugin::instance()->getModule( Visualizer_Module_Utility::NAME ); + $result = $utility->apply_global_style_settings( array(), $chart_id, 'pie' ); + + $this->assertCount( 2, $result['slices'], 'Palette size must match the legitimate rows in whitespace-wrapped chart data; object stubs are stripped.' ); + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'Utility pie palette call site must not instantiate objects from post_content.' + ); + } + + /** + * The ChartJS default-settings call site must not instantiate objects. + */ + public function test_utility_chartjs_defaults_call_site_is_guarded() { + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'auto-draft', + 'post_content' => wp_slash( + $this->object_payload( + array( + array( 'One' ), + array( 'Two' ), + ) + ) + ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_LIBRARY, 'ChartJS' ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'pie' ); + update_post_meta( + $chart_id, + Visualizer_Plugin::CF_SERIES, + array( + array( 'label' => 'Label', 'type' => 'string' ), + array( 'label' => 'Value', 'type' => 'number' ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, array() ); + + Visualizer_Module_Utility::set_defaults( get_post( $chart_id ) ); + $settings = get_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, true ); + + $this->assertCount( 2, $settings['slices'] ); + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'ChartJS defaults must not instantiate objects from post_content.' + ); + } + + /** + * The Gutenberg block render call site must not instantiate objects. + * + * Drives the front-end/REST data path with a requested chart that differs + * from the global post. Reverting the guard or reading the global post fails. + */ + public function test_gutenberg_block_render_call_site_is_guarded() { + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_content' => wp_slash( + $this->object_payload( + array( + array( 'requested-chart' ), + ) + ) + ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'line' ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_LIBRARY, 'ChartJS' ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, array() ); + update_post_meta( + $chart_id, + Visualizer_Plugin::CF_SERIES, + array( + array( 'label' => 'Label', 'type' => 'string' ), + ) + ); + + wp_set_current_user( self::factory()->user->create( array( 'role' => 'editor' ) ) ); + $decoy = self::factory()->post->create( + array( + 'post_content' => wp_slash( serialize( array( array( 'global-post' ) ) ) ), + ) + ); + $GLOBALS['post'] = get_post( $decoy ); + setup_postdata( $GLOBALS['post'] ); + + try { + $result = Visualizer_Gutenberg_Block::get_instance()->get_visualizer_data( array( 'id' => $chart_id ) ); + } finally { + wp_reset_postdata(); + } + + $this->assertIsArray( $result ); + $this->assertSame( 'requested-chart', $result['visualizer-data'][0][0] ); + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'Gutenberg block render call site must not instantiate objects from post_content.' + ); + } + + /** + * Cloning a chart copies raw post meta through maybe_decode_content(); it must + * neither instantiate objects from meta nor corrupt legitimate serialized meta. + */ + public function test_clone_chart_meta_is_guarded_and_round_trips() { + $settings = array( 'series' => array( array( 'color' => '#ff0000' ) ) ); + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_content' => wp_slash( serialize( array( array( 'Label' ), array( 'Value' ) ) ) ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, $settings ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SERIES, array( new Visualizer_POI_Canary() ) ); + Visualizer_POI_Canary::$awoke = false; + + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + $_GET['nonce'] = wp_create_nonce( Visualizer_Plugin::ACTION_CLONE_CHART ); + $_GET['chart'] = (string) $chart_id; + + try { + Visualizer_Plugin::instance()->getModule( Visualizer_Module_Chart::NAME )->cloneChart(); + } catch ( WPDieException $e ) { + // Expected test-mode exit before the redirect. + } + + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'Chart clone must not instantiate objects from raw post meta.' + ); + + $clones = get_posts( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => 'any', + 'exclude' => array( $chart_id ), + 'numberposts' => -1, + ) + ); + $this->assertCount( 1, $clones, 'Clone action must create exactly one new chart.' ); + $this->assertSame( + $settings, + get_post_meta( $clones[0]->ID, Visualizer_Plugin::CF_SETTINGS, true ), + 'Legitimate serialized meta must survive cloning unchanged.' + ); + } + + /** + * Saving and restoring a chart revision copies raw post meta through + * maybe_decode_content(); neither direction may instantiate objects, and + * legitimate serialized meta must survive the round trip. + */ + public function test_revision_meta_copy_is_guarded_and_round_trips() { + $settings = array( 'series' => array( array( 'color' => '#00ff00' ) ) ); + $chart_id = self::factory()->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_content' => wp_slash( serialize( array( array( 'Label' ), array( 'Value' ) ) ) ), + ) + ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, $settings ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SERIES, array( new Visualizer_POI_Canary() ) ); + Visualizer_POI_Canary::$awoke = false; + + // _wp_put_post_revision fires the hook that runs Admin::addRevision(). + $revision_id = _wp_put_post_revision( get_post( $chart_id ) ); + + $this->assertIsInt( $revision_id ); + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'Saving a revision must not instantiate objects from raw post meta.' + ); + $this->assertSame( + $settings, + get_metadata( 'post', $revision_id, Visualizer_Plugin::CF_SETTINGS, true ), + 'Legitimate serialized meta must be copied to the revision unchanged.' + ); + + // Change the live meta, then restore; wp_restore_post_revision fires + // the hook that runs Admin::restoreRevision(). + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, array( 'series' => array() ) ); + Visualizer_POI_Canary::$awoke = false; + + wp_restore_post_revision( $revision_id ); + + $this->assertFalse( + Visualizer_POI_Canary::$awoke, + 'Restoring a revision must not instantiate objects from raw revision meta.' + ); + $this->assertSame( + $settings, + get_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, true ), + 'Legitimate serialized meta must survive the revision restore unchanged.' + ); + } +} From 2bad5f1a2ff8cc5808f2f2de658447ecac866e01 Mon Sep 17 00:00:00 2001 From: Soare Robert Daniel Date: Wed, 15 Jul 2026 16:23:30 +0300 Subject: [PATCH 09/12] fix: preserve remote import compatibility (#1343) --- classes/Visualizer/Remote/Fetch.php | 53 +++++- tests/test-remote-fetch.php | 247 ++++++++++++++++++++++++++-- 2 files changed, 276 insertions(+), 24 deletions(-) diff --git a/classes/Visualizer/Remote/Fetch.php b/classes/Visualizer/Remote/Fetch.php index 92343580..03040c4c 100644 --- a/classes/Visualizer/Remote/Fetch.php +++ b/classes/Visualizer/Remote/Fetch.php @@ -74,9 +74,14 @@ public static function request( $url, $args = array() ) { if ( ! self::same_origin( $validated_url, $next_url ) ) { $args['headers'] = self::headers_for_cross_origin_redirect( isset( $args['headers'] ) ? $args['headers'] : array() ); unset( $args['cookies'] ); + } else { + $response_cookies = wp_remote_retrieve_cookies( $response ); + if ( ! empty( $response_cookies ) ) { + $args['cookies'] = array_merge( isset( $args['cookies'] ) ? $args['cookies'] : array(), $response_cookies ); + } } - if ( 303 === $status || ( in_array( $status, array( 301, 302 ), true ) && 'POST' === $args['method'] ) ) { + if ( in_array( $status, array( 302, 303 ), true ) ) { $args['method'] = 'GET'; unset( $args['body'] ); } @@ -142,12 +147,26 @@ public static function download( $url, $args = array() ) { */ private static function enforce_request_policy( $args ) { $args['method'] = strtoupper( (string) $args['method'] ); - if ( ! in_array( $args['method'], array( 'GET', 'POST' ), true ) ) { - return new WP_Error( 'visualizer_remote_method', 'Only GET and POST remote requests are allowed.' ); + if ( ! preg_match( '/^[!#$%&\'*+\-.^_`|~0-9A-Z]+$/', $args['method'] ) || in_array( $args['method'], array( 'CONNECT', 'TRACE' ), true ) ) { + return new WP_Error( 'visualizer_remote_method', 'The remote request method is not allowed.' ); + } + + if ( isset( $args['headers'] ) && is_string( $args['headers'] ) ) { + $headers = array(); + foreach ( preg_split( '/\r\n|\r|\n/', $args['headers'] ) as $header ) { + if ( false === strpos( $header, ':' ) ) { + continue; + } + list( $name, $value ) = explode( ':', $header, 2 ); + $headers[ strtolower( trim( $name ) ) ] = trim( $value ); + } + $args['headers'] = $headers; + } elseif ( ! isset( $args['headers'] ) || ! is_array( $args['headers'] ) ) { + $args['headers'] = array(); } $blocked_headers = array( 'connection', 'content-length', 'host', 'proxy-authorization', 'proxy-connection', 'te', 'trailer', 'transfer-encoding', 'upgrade' ); - foreach ( isset( $args['headers'] ) ? $args['headers'] : array() as $name => $value ) { + foreach ( $args['headers'] as $name => $value ) { if ( in_array( strtolower( (string) $name ), $blocked_headers, true ) ) { unset( $args['headers'][ $name ] ); } @@ -165,11 +184,12 @@ private static function enforce_request_policy( $args ) { * rebinding nameserver answer with a private address after validation * passed. Hostname requests fail closed when cURL pinning is unavailable. * - * @param string $url Validated URL. - * @param string[] $ips Validated addresses. + * @param string $url Validated URL. + * @param string[] $ips Validated addresses. + * @param string|null $curl_version Optional cURL version override. * @return callable|WP_Error|null The registered hook to remove after dispatch, an error when pinning is unavailable, or null for an IP literal or exempt host. */ - private static function pin_validated_addresses( $url, $ips ) { + private static function pin_validated_addresses( $url, $ips, $curl_version = null ) { if ( empty( $ips ) ) { return null; } @@ -183,18 +203,35 @@ private static function pin_validated_addresses( $url, $ips ) { return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely on this server.' ); } + $curl = curl_version(); if ( 'https' === strtolower( $parsed['scheme'] ) ) { - $curl = curl_version(); if ( empty( $curl['features'] ) || ! defined( 'CURL_VERSION_SSL' ) || ! ( $curl['features'] & CURL_VERSION_SSL ) ) { return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely on this server.' ); } } + if ( null === $curl_version ) { + $curl_version = isset( $curl['version'] ) ? $curl['version'] : '0.0.0'; + } $proxy = new WP_HTTP_Proxy(); if ( $proxy->is_enabled() && $proxy->send_through_proxy( $url ) ) { return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely through the configured proxy.' ); } + if ( version_compare( $curl_version, '7.59.0', '<' ) ) { + foreach ( $ips as $ip ) { + if ( filter_var( $ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4 ) ) { + $ips = array( $ip ); + break; + } + } + $ips = array( reset( $ips ) ); + } + + if ( version_compare( $curl_version, '7.57.0', '<' ) && filter_var( reset( $ips ), FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 ) ) { + return new WP_Error( 'visualizer_remote_transport', 'The remote host cannot be fetched securely on this server.' ); + } + $addresses = array(); foreach ( $ips as $ip ) { $addresses[] = filter_var( $ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6 ) ? '[' . $ip . ']' : $ip; diff --git a/tests/test-remote-fetch.php b/tests/test-remote-fetch.php index 8b6027b4..308ffc40 100644 --- a/tests/test-remote-fetch.php +++ b/tests/test-remote-fetch.php @@ -126,7 +126,8 @@ public function test_drops_cookies_on_cross_origin_redirect() { $filter = function ( $preempt, $args, $url ) use ( &$requests ) { $requests[] = array( $url, $args['cookies'] ); if ( 1 === count( $requests ) ) { - return $this->response( 302, array( 'location' => 'http://93.184.216.35/data' ) ); + $cookie = new WP_Http_Cookie( 'redirect=secret; Path=/', 'http://93.184.216.34/start' ); + return $this->response( 302, array( 'location' => 'http://93.184.216.35/data' ), '', array( $cookie ) ); } return $this->response( 200 ); }; @@ -144,6 +145,55 @@ public function test_drops_cookies_on_cross_origin_redirect() { $this->assertSame( array(), $requests[1][1] ); } + /** + * Cookies set by a response must be available to its same-origin redirect target. + */ + public function test_carries_response_cookies_on_same_origin_redirect() { + $requests = array(); + $filter = function ( $preempt, $args ) use ( &$requests ) { + $requests[] = $args['cookies']; + if ( 1 === count( $requests ) ) { + $cookie = new WP_Http_Cookie( 'session=secret; Path=/', 'http://93.184.216.34/start' ); + return $this->response( 302, array( 'location' => '/next' ), '', array( $cookie ) ); + } + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/start' ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertCount( 2, $requests ); + $this->assertCount( 1, $requests[1] ); + $this->assertSame( 'session', $requests[1][0]->name ); + } + + /** + * WordPress accepts request headers as a string, including across redirects. + */ + public function test_normalizes_string_headers_before_cross_origin_redirect() { + $requests = array(); + $filter = function ( $preempt, $args ) use ( &$requests ) { + $requests[] = $args['headers']; + if ( 1 === count( $requests ) ) { + return $this->response( 302, array( 'location' => 'http://93.184.216.35/next' ) ); + } + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::request( + 'http://93.184.216.34/start', + array( 'headers' => "Accept: application/json\r\nAuthorization: Bearer secret" ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertCount( 2, $requests ); + $this->assertSame( array( 'accept' => 'application/json' ), $requests[1] ); + } + /** * The validated addresses are pinned on the cURL transport only while the request dispatches. */ @@ -164,42 +214,133 @@ public function test_pins_validated_addresses_during_dispatch() { } /** - * IPv6 addresses use cURL's bracketed CURLOPT_RESOLVE syntax. + * Addresses use syntax supported by the installed cURL version. + * + * @dataProvider curl_resolve_entries + * @param string $curl_version cURL version. + * @param string[] $ips Validated addresses. + * @param string $expected Expected resolve entry. */ - public function test_formats_ipv6_addresses_for_curl_resolve() { + public function test_formats_addresses_for_curl_resolve( $curl_version, $ips, $expected ) { $method = new ReflectionMethod( Visualizer_Remote_Fetch::class, 'pin_validated_addresses' ); $method->setAccessible( true ); $pin = $method->invoke( null, 'https://example.com/data.json', - array( '93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946' ) + $ips, + $curl_version ); $this->assertIsCallable( $pin ); $reflection = new ReflectionFunction( $pin ); - $this->assertSame( - 'example.com:443:93.184.216.34,[2606:2800:220:1:248:1893:25c8:1946]', - $reflection->getStaticVariables()['entry'] - ); + $this->assertSame( $expected, $reflection->getStaticVariables()['entry'] ); remove_action( 'http_api_curl', $pin ); } + /** + * Resolve entries for supported cURL syntax generations. + * + * @return array[] + */ + public function curl_resolve_entries() { + return array( + 'modern mixed addresses' => array( + '7.59.0', + array( '93.184.216.34', '2606:2800:220:1:248:1893:25c8:1946' ), + 'example.com:443:93.184.216.34,[2606:2800:220:1:248:1893:25c8:1946]', + ), + 'legacy mixed addresses' => array( + '7.58.0', + array( '2606:2800:220:1:248:1893:25c8:1946', '93.184.216.34' ), + 'example.com:443:93.184.216.34', + ), + 'pre-IPv6 mixed addresses' => array( + '7.56.0', + array( '2606:2800:220:1:248:1893:25c8:1946', '93.184.216.34' ), + 'example.com:443:93.184.216.34', + ), + 'legacy IPv6 address' => array( + '7.57.0', + array( '2606:2800:220:1:248:1893:25c8:1946' ), + 'example.com:443:[2606:2800:220:1:248:1893:25c8:1946]', + ), + ); + } + + /** + * cURL versions without bracketed IPv6 resolve support fail closed for IPv6-only hosts. + */ + public function test_rejects_ipv6_pinning_on_unsupported_curl() { + $method = new ReflectionMethod( Visualizer_Remote_Fetch::class, 'pin_validated_addresses' ); + $method->setAccessible( true ); + $response = $method->invoke( + null, + 'https://example.com/data.json', + array( '2606:2800:220:1:248:1893:25c8:1946' ), + '7.56.0' + ); + + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_remote_transport', $response->get_error_code() ); + } + + /** + * JSON imports retain the established extended request methods. + * + * @dataProvider extended_json_request_methods + * @param string $method HTTP method. + */ + public function test_allows_extended_json_request_methods( $method ) { + $filter = function ( $preempt, $args ) use ( $method ) { + $this->assertSame( $method, $args['method'] ); + return $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/', array( 'method' => $method ) ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + } + + /** + * Extended JSON request methods. + * + * @return array[] + */ + public function extended_json_request_methods() { + return array( + 'PUT' => array( 'PUT' ), + 'PATCH' => array( 'PATCH' ), + 'DELETE' => array( 'DELETE' ), + 'HEAD' => array( 'HEAD' ), + 'OPTIONS' => array( 'OPTIONS' ), + 'PROPFIND' => array( 'PROPFIND' ), + ); + } + /** * Header and method policy is applied before dispatch. */ public function test_rejects_unsupported_method_before_request() { - $requests = 0; - $filter = function ( $preempt ) use ( &$requests ) { + $requests = 0; + $responses = array(); + $filter = function ( $preempt ) use ( &$requests ) { $requests++; - return $preempt; + return $this->response( 200 ); }; add_filter( 'pre_http_request', $filter ); - $response = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/', array( 'method' => 'DELETE' ) ); + foreach ( array( 'CONNECT', 'TRACE', "GET\r\nX-Injected: true" ) as $method ) { + $responses[] = Visualizer_Remote_Fetch::request( 'http://93.184.216.34/', array( 'method' => $method ) ); + } remove_filter( 'pre_http_request', $filter ); - $this->assertWPError( $response ); - $this->assertSame( 'visualizer_remote_method', $response->get_error_code() ); + + foreach ( $responses as $response ) { + $this->assertWPError( $response ); + $this->assertSame( 'visualizer_remote_method', $response->get_error_code() ); + } $this->assertSame( 0, $requests ); } @@ -257,6 +398,79 @@ public function test_same_origin_redirect_keeps_headers_and_resolves_relative_lo $this->assertSame( $headers, $requests[1][1] ); } + /** + * A 301 redirect preserves the request method and body, matching WordPress Requests. + */ + public function test_301_redirect_preserves_post_method_and_body() { + $requests = array(); + $filter = function ( $preempt, $args ) use ( &$requests ) { + $requests[] = array( $args['method'], isset( $args['body'] ) ? $args['body'] : null ); + return 1 === count( $requests ) + ? $this->response( 301, array( 'location' => '/next' ) ) + : $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::request( + 'http://93.184.216.34/start', + array( + 'method' => 'POST', + 'body' => '{"query":"data"}', + ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertSame( + array( + array( 'POST', '{"query":"data"}' ), + array( 'POST', '{"query":"data"}' ), + ), + $requests + ); + } + + /** + * Browser-compatible redirects switch extended methods to GET without a body. + * + * @dataProvider get_redirect_statuses + * @param int $status Redirect status. + */ + public function test_browser_redirect_switches_extended_method_to_get( $status ) { + $requests = array(); + $filter = function ( $preempt, $args ) use ( &$requests, $status ) { + $requests[] = array( $args['method'], isset( $args['body'] ) ? $args['body'] : null ); + return 1 === count( $requests ) + ? $this->response( $status, array( 'location' => '/next' ) ) + : $this->response( 200 ); + }; + add_filter( 'pre_http_request', $filter, 10, 2 ); + + $response = Visualizer_Remote_Fetch::request( + 'http://93.184.216.34/start', + array( + 'method' => 'PATCH', + 'body' => '{"query":"data"}', + ) + ); + + remove_filter( 'pre_http_request', $filter, 10 ); + $this->assertNotWPError( $response ); + $this->assertSame( array( 'GET', null ), $requests[1] ); + } + + /** + * Redirect statuses that WordPress follows with GET. + * + * @return array[] + */ + public function get_redirect_statuses() { + return array( + '302' => array( 302 ), + '303' => array( 303 ), + ); + } + /** * The redirect chain must stop after MAX_REDIRECTS hops. */ @@ -472,9 +686,10 @@ public function test_download_blocks_non_public_destination() { * @param int $code Status code. * @param array $headers Response headers. * @param string $body Response body. + * @param array $cookies Response cookies. * @return array */ - private function response( $code, $headers = array(), $body = '' ) { + private function response( $code, $headers = array(), $body = '', $cookies = array() ) { return array( 'headers' => $headers, 'body' => $body, @@ -482,7 +697,7 @@ private function response( $code, $headers = array(), $body = '' ) { 'code' => $code, 'message' => '', ), - 'cookies' => array(), + 'cookies' => $cookies, 'filename' => null, ); } From e42b4d7ab588b2543dd794c5669ea88fefee1cfd Mon Sep 17 00:00:00 2001 From: lucadobrescu Date: Wed, 15 Jul 2026 16:54:29 +0300 Subject: [PATCH 10/12] fix: enforce chart-level authorization on AJAX handlers (#1342) --- classes/Visualizer/Module.php | 21 +++ classes/Visualizer/Module/Admin.php | 2 +- classes/Visualizer/Module/Chart.php | 105 ++++++----- js/media/collection.js | 3 +- tests/test-ajax.php | 275 ++++++++++++++++++++++++++++ tests/test-export.php | 17 ++ 6 files changed, 377 insertions(+), 46 deletions(-) diff --git a/classes/Visualizer/Module.php b/classes/Visualizer/Module.php index 9d8166bb..99ac4a79 100644 --- a/classes/Visualizer/Module.php +++ b/classes/Visualizer/Module.php @@ -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. * diff --git a/classes/Visualizer/Module/Admin.php b/classes/Visualizer/Module/Admin.php index 7b9c1e5d..e003f2e6 100644 --- a/classes/Visualizer/Module/Admin.php +++ b/classes/Visualizer/Module/Admin.php @@ -437,7 +437,7 @@ public function setupMediaViewStrings( $strings ) { 'filters' => $chart_types, 'types' => array_keys( $chart_types ), ), - 'nonce' => wp_create_nonce(), + 'nonce' => wp_create_nonce( Visualizer_Plugin::ACTION_GET_CHARTS ), 'buildurl' => esc_url( add_query_arg( 'action', Visualizer_Plugin::ACTION_CREATE_CHART, admin_url( 'admin-ajax.php' ) ) ), ); diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index 65b9fcef..9e25d542 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -108,21 +108,24 @@ public function getSidebar( $sidebar, $chart_id ) { public function setJsonSchedule() { check_ajax_referer( Visualizer_Plugin::ACTION_JSON_SET_SCHEDULE . Visualizer_Plugin::VERSION, 'security' ); - $chart_id = filter_input( - INPUT_POST, - 'chart', + $chart_id = isset( $_POST['chart'] ) ? filter_var( + $_POST['chart'], FILTER_VALIDATE_INT, array( 'options' => array( 'min_range' => 1, ), ) - ); + ) : false; if ( ! $chart_id ) { wp_send_json_error(); } + if ( ! self::can_edit_chart( $chart_id ) ) { + wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ), 403 ); + } + $time = filter_input( INPUT_POST, 'time', @@ -232,10 +235,10 @@ public function setJsonData() { check_ajax_referer( Visualizer_Plugin::ACTION_JSON_SET_DATA . Visualizer_Plugin::VERSION, 'security' ); $params = $_POST; - $chart_id = $_GET['chart']; + $chart_id = isset( $_GET['chart'] ) ? absint( $_GET['chart'] ) : 0; - if ( empty( $chart_id ) ) { - wp_die(); + if ( ! self::can_edit_chart( $chart_id ) ) { + wp_die( esc_html__( 'You do not have permission to perform this action.', 'visualizer' ), '', array( 'response' => 403 ) ); } $chart = get_post( $chart_id ); @@ -319,6 +322,12 @@ public function setJsonData() { * @access public */ public function getCharts() { + check_ajax_referer( Visualizer_Plugin::ACTION_GET_CHARTS, 'nonce' ); + + if ( ! current_user_can( 'edit_posts' ) ) { + wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ), 403 ); + } + $query_args = array( 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, 'posts_per_page' => 9, @@ -334,6 +343,9 @@ public function getCharts() { ) ), ); + if ( ! current_user_can( 'edit_others_posts' ) ) { + $query_args['author'] = get_current_user_id(); + } $filter = filter_input( INPUT_GET, 's', FILTER_SANITIZE_STRING ); if ( empty( $filter ) ) { // 'filter' is from the modal from the add media button. @@ -454,24 +466,27 @@ public static function _sendResponse( $results ) { */ public function deleteChart() { $is_post = $_SERVER['REQUEST_METHOD'] === 'POST'; - $input_method = $is_post ? INPUT_POST : INPUT_GET; + $input = $is_post ? $_POST : $_GET; $chart_id = $success = false; - $nonce = wp_verify_nonce( filter_input( $input_method, 'nonce' ) ); - $capable = current_user_can( 'delete_posts' ); - if ( $nonce && $capable ) { - $chart_id = filter_input( - $input_method, - 'chart', + $nonce = isset( $input['nonce'] ) && wp_verify_nonce( $input['nonce'] ); + if ( $nonce ) { + $chart_id = isset( $input['chart'] ) ? filter_var( + $input['chart'], FILTER_VALIDATE_INT, array( 'options' => array( 'min_range' => 1, ), ) - ); + ) : false; if ( $chart_id ) { $chart = get_post( $chart_id ); - $success = $chart && $chart->post_type === Visualizer_Plugin::CPT_VISUALIZER; + $success = $chart + && $chart->post_type === Visualizer_Plugin::CPT_VISUALIZER + && ( + current_user_can( 'delete_post', $chart_id ) + || ( (int) $chart->post_author === get_current_user_id() && current_user_can( 'delete_posts' ) ) + ); } } if ( $success ) { @@ -554,6 +569,9 @@ public function renderChartPages() { $_POST = map_deep( $_POST, 'wp_strip_all_tags' ); } $chart = $chart_id ? get_post( $chart_id ) : null; + if ( $chart && ! self::can_edit_chart( $chart_id ) ) { + wp_die( esc_html__( 'You do not have permission to access this page.', 'visualizer' ), '', array( 'response' => 403 ) ); + } if ( ! $chart_id || ! $chart || $chart->post_type !== Visualizer_Plugin::CPT_VISUALIZER ) { if ( empty( $_GET['lang'] ) || empty( $_GET['parent_chart_id'] ) ) { $this->deleteOldCharts(); @@ -592,7 +610,7 @@ public function renderChartPages() { } else { $parent_chart_id = filter_var( $_GET['parent_chart_id'], FILTER_VALIDATE_INT ); $success = false; - if ( $parent_chart_id ) { + if ( $parent_chart_id && self::can_edit_chart( $parent_chart_id ) ) { $parent_chart = get_post( $parent_chart_id ); $success = $parent_chart && $parent_chart->post_type === Visualizer_Plugin::CPT_VISUALIZER; } @@ -800,14 +818,14 @@ private function handlePermissions() { * Handle data and settings page */ private function _handleDataAndSettingsPage() { - if ( isset( $_POST['map_api_key'] ) ) { - update_option( 'visualizer-map-api-key', $_POST['map_api_key'] ); - } - if ( $_SERVER['REQUEST_METHOD'] === 'POST' && isset( $_GET['nonce'] ) && wp_verify_nonce( $_GET['nonce'] ) ) { $is_canceled = isset( $_POST['cancel'] ) && 1 === intval( $_POST['cancel'] ); $is_newly_created = $this->_chart->post_status === 'auto-draft'; + if ( isset( $_POST['map_api_key'] ) && current_user_can( 'manage_options' ) ) { + update_option( 'visualizer-map-api-key', sanitize_text_field( wp_unslash( $_POST['map_api_key'] ) ) ); + } + if ( $is_newly_created && ! $is_canceled ) { $this->_chart->post_status = 'publish'; @@ -1431,10 +1449,9 @@ public function uploadData() { public function cloneChart() { $chart_id = $success = false; $nonce = isset( $_GET['nonce'] ) && wp_verify_nonce( $_GET['nonce'], Visualizer_Plugin::ACTION_CLONE_CHART ); - $capable = current_user_can( 'edit_posts' ); - if ( $nonce && $capable ) { + if ( $nonce ) { $chart_id = isset( $_GET['chart'] ) ? filter_var( $_GET['chart'], FILTER_VALIDATE_INT ) : ''; - if ( $chart_id ) { + if ( $chart_id && self::can_edit_chart( $chart_id ) ) { $chart = get_post( $chart_id ); $success = $chart && $chart->post_type === Visualizer_Plugin::CPT_VISUALIZER; } @@ -1490,22 +1507,19 @@ public function cloneChart() { */ public function exportData() { check_ajax_referer( Visualizer_Plugin::ACTION_EXPORT_DATA . Visualizer_Plugin::VERSION, 'security' ); - $capable = current_user_can( 'edit_posts' ); - if ( $capable ) { - $chart_id = isset( $_GET['chart'] ) ? filter_var( - $_GET['chart'], - FILTER_VALIDATE_INT, - array( - 'options' => array( - 'min_range' => 1, - ), - ) - ) : ''; - if ( $chart_id ) { - $data = $this->_getDataAs( $chart_id, 'csv' ); - if ( $data ) { - echo wp_send_json_success( $data ); - } + $chart_id = isset( $_GET['chart'] ) ? filter_var( + $_GET['chart'], + FILTER_VALIDATE_INT, + array( + 'options' => array( + 'min_range' => 1, + ), + ) + ) : ''; + if ( $chart_id && self::can_edit_chart( $chart_id ) ) { + $data = $this->_getDataAs( $chart_id, 'csv' ); + if ( $data ) { + echo wp_send_json_success( $data ); } } @@ -1687,16 +1701,19 @@ public function saveQuery() { public function saveFilter() { check_ajax_referer( Visualizer_Plugin::ACTION_SAVE_FILTER_QUERY . Visualizer_Plugin::VERSION, 'security' ); - $chart_id = filter_input( - INPUT_GET, - 'chart', + $chart_id = isset( $_GET['chart'] ) ? filter_var( + $_GET['chart'], FILTER_VALIDATE_INT, array( 'options' => array( 'min_range' => 1, ), ) - ); + ) : false; + + if ( ! self::can_edit_chart( $chart_id ) ) { + wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ), 403 ); + } $hours = filter_input( INPUT_POST, diff --git a/js/media/collection.js b/js/media/collection.js index a24e3c0c..fe1d60e8 100644 --- a/js/media/collection.js +++ b/js/media/collection.js @@ -7,7 +7,8 @@ options = options || {}; options.type = 'GET'; options.data = _.extend( options.data || {}, { - action: wp.media.view.l10n.visualizer.actions.get_charts + action: wp.media.view.l10n.visualizer.actions.get_charts, + nonce: wp.media.view.l10n.visualizer.nonce }); return wp.media.ajax( options ); diff --git a/tests/test-ajax.php b/tests/test-ajax.php index 392fbc49..1e39e296 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -635,6 +635,250 @@ public function test_json_get_roots_blocks_link_local_for_contributor() { $this->assertSame( 0, $requests ); } + /** + * Chart authorization validates both the object type and meta capability. + */ + public function test_chart_authorization_is_object_specific() { + $own_chart = $this->create_chart_for_user( $this->contibutor_user_id, 'publish' ); + $other_chart = $this->create_chart_for_user( $this->admin_user_id ); + $regular_post = $this->factory->post->create( + array( + 'post_author' => $this->contibutor_user_id, + ) + ); + wp_set_current_user( $this->contibutor_user_id ); + + $this->assertTrue( Visualizer_Module::can_edit_chart( $own_chart ) ); + $this->assertFalse( Visualizer_Module::can_edit_chart( $other_chart ) ); + $this->assertFalse( Visualizer_Module::can_edit_chart( $regular_post ) ); + $this->assertFalse( Visualizer_Module::can_edit_chart( PHP_INT_MAX ) ); + } + + /** + * The chart editor rejects another user's chart before saving settings. + */ + public function test_edit_chart_denied_for_chart_user_cannot_edit() { + $chart_id = $this->create_chart_for_user( $this->admin_user_id ); + $original = array( 'title' => 'Original' ); + update_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, $original ); + wp_set_current_user( $this->contibutor_user_id ); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_GET = array( + 'chart' => $chart_id, + 'tab' => 'settings', + 'nonce' => wp_create_nonce(), + ); + $_POST = array( + 'title' => 'Overwritten', + 'save' => 1, + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_EDIT_CHART ); + $this->fail( 'Expected the request to die for a chart the user cannot edit.' ); + } catch ( WPAjaxDieStopException $e ) { + $this->assertStringContainsString( 'permission', $e->getMessage() ); + } + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $this->assertSame( $original, get_post_meta( $chart_id, Visualizer_Plugin::CF_SETTINGS, true ) ); + } + + /** + * Chart editors cannot update the site-wide map API key. + */ + public function test_edit_chart_map_api_key_requires_manage_options() { + $chart_id = $this->create_chart_for_user( $this->contibutor_user_id ); + add_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'line' ); + update_option( 'visualizer-map-api-key', 'original-key' ); + wp_set_current_user( $this->contibutor_user_id ); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_GET = array( + 'chart' => $chart_id, + 'tab' => 'settings', + 'nonce' => wp_create_nonce(), + ); + $_POST = array( + 'map_api_key' => 'attacker-key', + 'save' => 1, + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_EDIT_CHART ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected when the editor response completes. + } + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $this->assertSame( 'original-key', get_option( 'visualizer-map-api-key' ) ); + } + + /** + * Contributors only receive charts they own from the library endpoint. + */ + public function test_get_charts_only_lists_editable_scope_for_contributor() { + $own_chart = $this->create_chart_for_user( $this->contibutor_user_id, 'publish' ); + $other_chart = $this->create_chart_for_user( $this->admin_user_id, 'publish' ); + wp_set_current_user( $this->contibutor_user_id ); + $_GET = array( + 'nonce' => wp_create_nonce( Visualizer_Plugin::ACTION_GET_CHARTS ), + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_GET_CHARTS ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after the JSON response. + } + + $response = json_decode( $this->_last_response, true ); + $chart_ids = wp_list_pluck( $response['data'], 'id' ); + $this->assertTrue( $response['success'] ); + $this->assertContains( $own_chart, $chart_ids ); + $this->assertNotContains( $other_chart, $chart_ids ); + } + + /** + * A valid nonce does not permit deleting another user's chart. + */ + public function test_delete_chart_denied_for_chart_user_cannot_delete() { + $chart_id = $this->create_chart_for_user( $this->admin_user_id ); + wp_set_current_user( $this->contibutor_user_id ); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_POST = array( + 'chart' => $chart_id, + 'nonce' => wp_create_nonce(), + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_DELETE_CHART ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after the JSON response. + } + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $response = json_decode( $this->_last_response ); + $this->assertFalse( $response->success ); + $this->assertNotNull( get_post( $chart_id ) ); + } + + /** + * A contributor may delete their own published chart. + */ + public function test_delete_chart_allows_owner_without_delete_published_posts() { + $chart_id = $this->create_chart_for_user( $this->contibutor_user_id, 'publish' ); + wp_set_current_user( $this->contibutor_user_id ); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_POST = array( + 'chart' => $chart_id, + 'nonce' => wp_create_nonce(), + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_DELETE_CHART ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after the JSON response. + } + $_SERVER['REQUEST_METHOD'] = 'GET'; + + $response = json_decode( $this->_last_response ); + $this->assertTrue( $response->success ); + $this->assertNull( get_post( $chart_id ) ); + } + + /** + * A valid nonce does not permit cloning another user's chart. + */ + public function test_clone_chart_denied_for_chart_user_cannot_edit() { + $chart_id = $this->create_chart_for_user( $this->admin_user_id ); + wp_set_current_user( $this->contibutor_user_id ); + $_GET = array( + 'chart' => $chart_id, + 'nonce' => wp_create_nonce( Visualizer_Plugin::ACTION_CLONE_CHART ), + ); + $before = wp_count_posts( Visualizer_Plugin::CPT_VISUALIZER )->draft; + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_CLONE_CHART ); + $this->fail( 'Expected the request to die for a chart the user cannot edit.' ); + } catch ( WPAjaxDieStopException $e ) { + // cloneChart() ends with wp_die() in the test environment when denied. + } catch ( WPAjaxDieContinueException $e ) { + // Expected when the handler ends normally. + } + + $this->assertSame( $before, wp_count_posts( Visualizer_Plugin::CPT_VISUALIZER )->draft ); + } + + /** + * A user cannot alter another user's JSON refresh schedule. + */ + public function test_json_schedule_denied_for_chart_user_cannot_edit() { + $chart_id = $this->create_chart_for_user( $this->admin_user_id ); + wp_set_current_user( $this->contibutor_user_id ); + $_POST = array( + 'chart' => $chart_id, + 'time' => 12, + 'security' => wp_create_nonce( Visualizer_Plugin::ACTION_JSON_SET_SCHEDULE . Visualizer_Plugin::VERSION ), + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_SET_SCHEDULE ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after the JSON response. + } + + $response = json_decode( $this->_last_response ); + $this->assertFalse( $response->success ); + $this->assertSame( '', get_post_meta( $chart_id, Visualizer_Plugin::CF_JSON_SCHEDULE, true ) ); + } + + /** + * A user cannot replace another user's JSON chart data. + */ + public function test_json_set_data_denied_for_chart_user_cannot_edit() { + $chart_id = $this->create_chart_for_user( $this->admin_user_id ); + wp_set_current_user( $this->contibutor_user_id ); + $_GET = array( + 'chart' => $chart_id, + 'security' => wp_create_nonce( Visualizer_Plugin::ACTION_JSON_SET_DATA . Visualizer_Plugin::VERSION ), + ); + $_POST = array( + 'url' => 'https://example.com/data.json', + 'method' => 'GET', + 'root' => 'items', + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_SET_DATA ); + $this->fail( 'Expected the request to die for a chart the user cannot edit.' ); + } catch ( WPAjaxDieStopException $e ) { + $this->assertStringContainsString( 'permission', $e->getMessage() ); + } + + $this->assertSame( '', get_post_meta( $chart_id, Visualizer_Plugin::CF_JSON_URL, true ) ); + } + + /** + * A user cannot save filters on another user's chart. + */ + public function test_save_filter_denied_for_chart_user_cannot_edit() { + $chart_id = $this->create_chart_for_user( $this->admin_user_id ); + wp_set_current_user( $this->contibutor_user_id ); + $_GET = array( + 'chart' => $chart_id, + 'security' => wp_create_nonce( Visualizer_Plugin::ACTION_SAVE_FILTER_QUERY . Visualizer_Plugin::VERSION ), + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_SAVE_FILTER_QUERY ); + } catch ( WPAjaxDieContinueException $e ) { + // Expected after the JSON response. + } + + $response = json_decode( $this->_last_response ); + $this->assertFalse( $response->success ); + } + /** * Test that the setup wizard import step builds per-row settings from the * decoded sample data, covering the decode_content() call in the wizard. @@ -667,6 +911,37 @@ public function test_wizard_import_chart_builds_settings_from_decoded_sample_dat $this->assertCount( $expected_rows, $settings['slices'] ); } + /** + * Creates a chart owned by a specific user. + * + * @param int $user_id User ID. + * @param string $status Post status. + * @return int + */ + private function create_chart_for_user( $user_id, $status = 'draft' ) { + $chart_id = $this->factory->post->create( + array( + 'post_type' => Visualizer_Plugin::CPT_VISUALIZER, + 'post_status' => $status, + 'post_author' => $user_id, + 'post_content' => wp_slash( serialize( array( array( 'Label' ), array( 'Value' ) ) ) ), + ) + ); + add_post_meta( $chart_id, Visualizer_Plugin::CF_CHART_TYPE, 'line' ); + add_post_meta( + $chart_id, + Visualizer_Plugin::CF_SERIES, + array( + array( + 'label' => 'Label', + 'type' => 'string', + ), + ) + ); + + return $chart_id; + } + /** * Utility method to mock pro version. */ diff --git a/tests/test-export.php b/tests/test-export.php index 7f47aeb6..3c56887f 100644 --- a/tests/test-export.php +++ b/tests/test-export.php @@ -163,4 +163,21 @@ public function test_csv_export_invalid_chart_returns_no_success() { $response = json_decode( $this->_last_response ); $this->assertTrue( empty( $response ) || empty( $response->success ) ); } + + /** + * Exporting requires permission to edit the requested chart. + */ + public function test_csv_export_denied_for_chart_user_cannot_edit() { + $this->create_chart(); + $user_id = $this->factory->user->create( + array( + 'role' => 'contributor', + ) + ); + wp_set_current_user( $user_id ); + + $response = $this->run_export(); + + $this->assertTrue( empty( $response ) || empty( $response->success ) ); + } } From fd96ea1c617f5d324c59fe8df324f5f4854b425a Mon Sep 17 00:00:00 2001 From: Soare Robert Daniel Date: Thu, 16 Jul 2026 12:42:29 +0300 Subject: [PATCH 11/12] fix: prevent D3 export action XSS via untrusted dataUrl (#1344) --- classes/Visualizer/D3Renderer/src/index.js | 25 +++- .../D3Renderer/tests/print-action-xss.test.js | 122 ++++++++++++++++++ classes/Visualizer/Module/Chart.php | 16 ++- phpstan-baseline.neon | 18 --- tests/test-sanitize-settings.php | 31 +++++ 5 files changed, 187 insertions(+), 25 deletions(-) create mode 100644 classes/Visualizer/D3Renderer/tests/print-action-xss.test.js diff --git a/classes/Visualizer/D3Renderer/src/index.js b/classes/Visualizer/D3Renderer/src/index.js index a8344f5b..56feb472 100644 --- a/classes/Visualizer/D3Renderer/src/index.js +++ b/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( "
" ); + 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 { diff --git a/classes/Visualizer/D3Renderer/tests/print-action-xss.test.js b/classes/Visualizer/D3Renderer/tests/print-action-xss.test.js new file mode 100644 index 00000000..bc2bce7e --- /dev/null +++ b/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