Skip to content

Commit fa41530

Browse files
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 <noreply@anthropic.com>
1 parent aa36ff4 commit fa41530

3 files changed

Lines changed: 115 additions & 1 deletion

File tree

classes/Visualizer/Module/Chart.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ public function setJsonSchedule() {
170170
public function getJsonRoots() {
171171
check_ajax_referer( Visualizer_Plugin::ACTION_JSON_GET_ROOTS . Visualizer_Plugin::VERSION, 'security' );
172172

173+
if ( ! current_user_can( 'edit_posts' ) ) {
174+
wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ) );
175+
}
176+
173177
$params = wp_parse_args( $_POST['params'] );
174178

175179
$source = new Visualizer_Source_Json( $params );
@@ -192,6 +196,10 @@ public function getJsonRoots() {
192196
public function getJsonData() {
193197
check_ajax_referer( Visualizer_Plugin::ACTION_JSON_GET_DATA . Visualizer_Plugin::VERSION, 'security' );
194198

199+
if ( ! current_user_can( 'edit_posts' ) ) {
200+
wp_send_json_error( array( 'msg' => esc_html__( 'You do not have permission to perform this action.', 'visualizer' ) ) );
201+
}
202+
195203
$params = wp_parse_args( $_POST['params'] );
196204

197205
$chart_id = $params['chart'];

classes/Visualizer/Source/Json.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ function ( $headers ) {
468468
}
469469

470470
do_action( 'themeisle_log_event', Visualizer_Plugin::NAME, sprintf( 'Connecting to %s with args = %s ', $url, print_r( $args, true ) ), 'debug', __FILE__, __LINE__ );
471-
return wp_remote_request( $url, $args );
471+
return wp_safe_remote_request( $url, $args );
472472
}
473473

474474
/**

tests/test-ajax.php

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,112 @@ public function test_sql_save_chart_admin() {
330330
$this->assertFalse( $response->success );
331331
}
332332

333+
/**
334+
* JSON get-roots must be denied for users without `edit_posts` (issue #591 access-control fix).
335+
*/
336+
public function test_json_get_roots_denied_for_subscriber() {
337+
wp_set_current_user( $this->subscriber_user_id );
338+
$this->_setRole( 'subscriber' );
339+
340+
$_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_ROOTS . Visualizer_Plugin::VERSION );
341+
$_POST['params'] = array(
342+
'url' => 'http://127.0.0.1:9999/latest/meta-data/',
343+
'method' => 'GET',
344+
);
345+
346+
try {
347+
$this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_ROOTS );
348+
} catch ( WPAjaxDieContinueException $e ) {
349+
// We expected this, do nothing.
350+
}
351+
352+
$response = json_decode( $this->_last_response );
353+
$this->assertIsObject( $response );
354+
$this->assertFalse( $response->success );
355+
$this->assertEquals( 'You do not have permission to perform this action.', $response->data->msg );
356+
}
357+
358+
/**
359+
* JSON get-data must be denied for users without `edit_posts` (issue #591 access-control fix).
360+
*/
361+
public function test_json_get_data_denied_for_subscriber() {
362+
wp_set_current_user( $this->subscriber_user_id );
363+
$this->_setRole( 'subscriber' );
364+
365+
$_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_DATA . Visualizer_Plugin::VERSION );
366+
$_POST['params'] = array(
367+
'url' => 'http://127.0.0.1:9999/',
368+
'method' => 'GET',
369+
'chart' => 1,
370+
);
371+
372+
try {
373+
$this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_DATA );
374+
} catch ( WPAjaxDieContinueException $e ) {
375+
// We expected this, do nothing.
376+
}
377+
378+
$response = json_decode( $this->_last_response );
379+
$this->assertIsObject( $response );
380+
$this->assertFalse( $response->success );
381+
$this->assertEquals( 'You do not have permission to perform this action.', $response->data->msg );
382+
}
383+
384+
/**
385+
* A permitted user (contributor) is not blocked by the guard, and the JSON source fetches through the
386+
* SSRF-safe transport (`reject_unsafe_urls`), matching the CSV path (issue #591 SSRF fix).
387+
*/
388+
public function test_json_get_roots_allowed_for_contributor_uses_safe_transport() {
389+
wp_set_current_user( $this->contibutor_user_id );
390+
$this->_setRole( 'contributor' );
391+
392+
$captured = array();
393+
add_filter(
394+
'pre_http_request',
395+
function ( $pre, $args, $url ) use ( &$captured ) {
396+
$captured[] = array(
397+
'url' => $url,
398+
'reject' => ! empty( $args['reject_unsafe_urls'] ),
399+
);
400+
return array(
401+
'headers' => array(),
402+
'body' => wp_json_encode( array( 'results' => array( array( 'id' => 1 ) ) ) ),
403+
'response' => array(
404+
'code' => 200,
405+
'message' => 'OK',
406+
),
407+
'cookies' => array(),
408+
'filename' => null,
409+
);
410+
},
411+
10,
412+
3
413+
);
414+
415+
$_GET['security'] = wp_create_nonce( Visualizer_Plugin::ACTION_JSON_GET_ROOTS . Visualizer_Plugin::VERSION );
416+
$_POST['params'] = array(
417+
'url' => 'http://127.0.0.1:9999/latest/meta-data/',
418+
'method' => 'GET',
419+
);
420+
421+
try {
422+
$this->_handleAjax( Visualizer_Plugin::ACTION_JSON_GET_ROOTS );
423+
} catch ( WPAjaxDieContinueException $e ) {
424+
// We expected this, do nothing.
425+
}
426+
427+
$response = json_decode( $this->_last_response );
428+
$this->assertIsObject( $response );
429+
// The capability guard must NOT block a user who has edit_posts.
430+
$msg = isset( $response->data->msg ) ? $response->data->msg : '';
431+
$this->assertNotEquals( 'You do not have permission to perform this action.', $msg );
432+
// Every outbound request for the JSON source must use the SSRF-safe transport.
433+
$this->assertNotEmpty( $captured, 'The JSON source did not attempt any fetch.' );
434+
foreach ( $captured as $req ) {
435+
$this->assertTrue( $req['reject'], 'JSON fetch must use wp_safe_remote_* (reject_unsafe_urls => true).' );
436+
}
437+
}
438+
333439
/**
334440
* Utility method to mock pro version.
335441
*/

0 commit comments

Comments
 (0)