From fa41530b5916782d11c457339639198c899fc978 Mon Sep 17 00:00:00 2001 From: RaduCristianPopescu <119046336+RaduCristianPopescu@users.noreply.github.com> Date: Wed, 8 Jul 2026 10:44:34 +0300 Subject: [PATCH] fix: prevent SSRF via JSON import handlers The `getJsonRoots()` and `getJsonData()` AJAX handlers verified only a nonce before fetching a user-supplied URL server-side, and the fetch used the unsafe `wp_remote_request()`. A Contributor (edit_posts) could read the nonce from the chart editor and make WordPress fetch arbitrary internal URLs, with the response reflected back (non-blind SSRF). - Add a `current_user_can( 'edit_posts' )` guard to both handlers, matching the sibling AJAX handlers in the file. - Switch `Visualizer_Source_Json::connect()` to `wp_safe_remote_request()`, matching the SSRF-safe transport already used by the CSV/remote path. - Add regression tests covering the capability guard and safe transport. Fixes Codeinwp/visualizer-pro#591 Co-Authored-By: Claude Fable 5 --- classes/Visualizer/Module/Chart.php | 8 +++ classes/Visualizer/Source/Json.php | 2 +- tests/test-ajax.php | 106 ++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/classes/Visualizer/Module/Chart.php b/classes/Visualizer/Module/Chart.php index fc3190f4a..f71f1fc9a 100644 --- a/classes/Visualizer/Module/Chart.php +++ b/classes/Visualizer/Module/Chart.php @@ -170,6 +170,10 @@ public function setJsonSchedule() { public function getJsonRoots() { check_ajax_referer( Visualizer_Plugin::ACTION_JSON_GET_ROOTS . Visualizer_Plugin::VERSION, 'security' ); + if ( ! current_user_can( 'edit_posts' ) ) { + wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ) ); + } + $params = wp_parse_args( $_POST['params'] ); $source = new Visualizer_Source_Json( $params ); @@ -192,6 +196,10 @@ public function getJsonRoots() { public function getJsonData() { check_ajax_referer( Visualizer_Plugin::ACTION_JSON_GET_DATA . Visualizer_Plugin::VERSION, 'security' ); + if ( ! current_user_can( 'edit_posts' ) ) { + wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ) ); + } + $params = wp_parse_args( $_POST['params'] ); $chart_id = $params['chart']; diff --git a/classes/Visualizer/Source/Json.php b/classes/Visualizer/Source/Json.php index a62ac137b..5edf29d19 100644 --- a/classes/Visualizer/Source/Json.php +++ b/classes/Visualizer/Source/Json.php @@ -468,7 +468,7 @@ function ( $headers ) { } 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_remote_request( $url, $args ); + return wp_safe_remote_request( $url, $args ); } /** diff --git a/tests/test-ajax.php b/tests/test-ajax.php index 1b197b603..20b1a26f1 100644 --- a/tests/test-ajax.php +++ b/tests/test-ajax.php @@ -330,6 +330,112 @@ public function test_sql_save_chart_admin() { $this->assertFalse( $response->success ); } + /** + * JSON get-roots must be denied for users without `edit_posts` (issue #591 access-control fix). + */ + public function test_json_get_roots_denied_for_subscriber() { + wp_set_current_user( $this->subscriber_user_id ); + $this->_setRole( 'subscriber' ); + + $_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/', + 'method' => 'GET', + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_ROOTS ); + } catch ( WPAjaxDieContinueException $e ) { + // We expected this, do nothing. + } + + $response = json_decode( $this->_last_response ); + $this->assertIsObject( $response ); + $this->assertFalse( $response->success ); + $this->assertEquals( 'You do not have permission to perform this action.', $response->data->msg ); + } + + /** + * JSON get-data must be denied for users without `edit_posts` (issue #591 access-control fix). + */ + public function test_json_get_data_denied_for_subscriber() { + wp_set_current_user( $this->subscriber_user_id ); + $this->_setRole( 'subscriber' ); + + $_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_DATA . Visualizer_Plugin::VERSION ); + $_POST['params'] = array( + 'url' => 'http://127.0.0.1:9999/', + 'method' => 'GET', + 'chart' => 1, + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_DATA ); + } catch ( WPAjaxDieContinueException $e ) { + // We expected this, do nothing. + } + + $response = json_decode( $this->_last_response ); + $this->assertIsObject( $response ); + $this->assertFalse( $response->success ); + $this->assertEquals( 'You do not have permission to perform this action.', $response->data->msg ); + } + + /** + * 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). + */ + public function test_json_get_roots_allowed_for_contributor_uses_safe_transport() { + wp_set_current_user( $this->contibutor_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 + ); + + $_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/', + 'method' => 'GET', + ); + + try { + $this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_ROOTS ); + } catch ( WPAjaxDieContinueException $e ) { + // We expected this, do nothing. + } + + $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).' ); + } + } + /** * Utility method to mock pro version. */