Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions admin/class-boldgrid-backup-admin-auto-rollback.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ public function cancel() {

// Remove WP option boldgrid_backup_pending_rollback.
$this->core->settings->delete_rollback_option();

// Remove the one-time CLI cancel secret.
delete_site_option( 'boldgrid_backup_cli_cancel_secret' );
}

/**
Expand Down Expand Up @@ -1194,16 +1197,22 @@ public function validate_rollback_option( $value, $option ) {
/**
* Callback function for canceling a pending rollback from the cli process.
*
* This admin-ajax call is unprovileged, so that the CLI script can make the call.
* The only validation that we use is the backup identifier.
* Nobody will be trying to cancel rollbacks (with a 15-minute window) anyways.
* This admin-ajax call is unprivileged, so that the CLI script can make the call.
* Validation requires both the backup identifier and a one-time random secret that
* was generated when the restore cron job was scheduled.
*
* @since 1.10.7
*/
public function wp_ajax_cli_cancel() {
$backup_id_match = ! empty( $_GET['backup_id'] ) && $this->core->get_backup_identifier() === sanitize_key( $_GET['backup_id'] ); // phpcs:ignore WordPress.CSRF.NonceVerification.NoNonceVerification
// phpcs:ignore WordPress.CSRF.NonceVerification.Recommended
$backup_id_match = ! empty( $_GET['backup_id'] ) && $this->core->get_backup_identifier() === sanitize_key( $_GET['backup_id'] );

$stored_secret = get_site_option( 'boldgrid_backup_cli_cancel_secret', '' );
// phpcs:ignore WordPress.CSRF.NonceVerification.Recommended
$secret_match = ! empty( $stored_secret ) && ! empty( $_GET['cli_cancel_secret'] ) &&
hash_equals( $stored_secret, sanitize_text_field( wp_unslash( $_GET['cli_cancel_secret'] ) ) );

if ( $backup_id_match ) {
if ( $backup_id_match && $secret_match ) {
$this->cancel();
wp_send_json_success( __( 'Rollback canceled', 'boldgrid-backup' ) );
} else {
Expand Down
5 changes: 5 additions & 0 deletions admin/class-boldgrid-backup-admin-cron.php
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,10 @@ public function get_restore_command() {
$settings = $this->core->settings->get_settings();
$backup_id = $this->core->get_backup_identifier();

// Generate and store a one-time random secret for the CLI cancel endpoint.
$cli_cancel_secret = wp_generate_password( 32, false );
update_site_option( 'boldgrid_backup_cli_cancel_secret', $cli_cancel_secret );

$entry_parts = [
date( $time['minute'] . ' ' . $time['hour'], $time['deadline'] ) . ' * * ' . date( 'w' ),
$this->cron_command,
Expand All @@ -795,6 +799,7 @@ public function get_restore_command() {
'mode=restore restore',
'notify email=' . $settings['notification_email'],
'backup_id=' . $backup_id,
'cli_cancel_secret=' . $cli_cancel_secret,
'zip=' . $this->core->archive->filepath,
];

Expand Down
3 changes: 2 additions & 1 deletion cli/class-site-restore.php
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ private function increment_restore_attempts() {
private function cancel_rollback() {
require_once dirname( dirname( __FILE__ ) ) . '/cron/class-boldgrid-backup-url-helper.php';
$url = Info::get_info()['siteurl'] . '/wp-admin/admin-ajax.php?action=boldgrid_cli_cancel_rollback&backup_id=' .
Info::get_arg_value( 'backup_id' );
rawurlencode( (string) Info::get_arg_value( 'backup_id' ) ) .
'&cli_cancel_secret=' . rawurlencode( (string) Info::get_arg_value( 'cli_cancel_secret' ) );
$success = ( new \Boldgrid_Backup_Url_Helper() )->call_url( $url, $status, $errorno, $error );
}
}
109 changes: 95 additions & 14 deletions tests/admin/test-class-boldgrid-backup-admin-ajax.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*
* @group ajax
* @runTestsInSeparateProcesses
* @preserveGlobalState disabled
*/
class Test_Boldgrid_Backup_Admin_Ajax extends WP_Ajax_UnitTestCase {
/**
Expand All @@ -36,14 +37,52 @@ public function set_up() {
$this->core = new Boldgrid_Backup_Admin_Core();
}

/**
* Simulate the server-side state created when a restore cron job is scheduled.
*
* Stores a known secret in the site option and returns it so tests can pass it
* as the cli_cancel_secret GET parameter.
*
* @since 1.17.2
*
* @param string $secret Optional secret value to store. Defaults to a fixed test value.
* @return string The secret that was stored.
*/
private function set_cli_cancel_secret( $secret = 'test_cli_cancel_secret_abc123' ) {
update_site_option( 'boldgrid_backup_cli_cancel_secret', $secret );
return $secret;
}

/**
* Assert that an AJAX response represents a failure with "Invalid arguments".
*
* @since 1.17.2
*/
private function assertInvalidArgumentsResponse() {
// The pending rollback option should not have been cleared.
$this->assertEquals( 'test', get_option( 'boldgrid_backup_pending_rollback' ) );

$response = json_decode( $this->_last_response );

$this->assertInternalType( 'object', $response );
$this->assertObjectHasAttribute( 'success', $response );
$this->assertFalse( $response->success );
$this->assertObjectHasAttribute( 'data', $response );
$this->assertEquals( 'Invalid arguments', $response->data );
}

/**
* Test wp_ajax_cli_cancel for success.
*
* Both a valid backup_id and a valid cli_cancel_secret (matching the stored
* site option) are required for the cancel to succeed.
*
* @since 1.10.7
*/
public function test_wp_ajax_cli_cancel_success() {
global $_GET; // phpcs:ignore
$_GET['backup_id'] = $this->core->get_backup_identifier();
$_GET['backup_id'] = $this->core->get_backup_identifier();
$_GET['cli_cancel_secret'] = $this->set_cli_cancel_secret();

update_option( 'boldgrid_backup_pending_rollback', 'test' );

Expand All @@ -54,9 +93,12 @@ public function test_wp_ajax_cli_cancel_success() {
echo null;
}

// This option was given a value above, and the cancel rollback method should have deleted it.
// The rollback option should have been cleared by cancel().
$this->assertEmpty( get_option( 'boldgrid_backup_pending_rollback' ) );

// The one-time secret should have been deleted by cancel().
$this->assertFalse( get_site_option( 'boldgrid_backup_cli_cancel_secret', false ) );

$response = json_decode( $this->_last_response );

$this->assertInternalType( 'object', $response ); // Should be an object.
Expand All @@ -67,13 +109,14 @@ public function test_wp_ajax_cli_cancel_success() {
}

/**
* Test wp_ajax_cli_cancel for failure.
* Test wp_ajax_cli_cancel fails when backup_id is missing.
*
* @since 1.10.7
*/
public function test_wp_ajax_cli_cancel_failure() {
global $_GET; // phpcs:ignore
unset( $_GET['backup_id'] );
$_GET['cli_cancel_secret'] = $this->set_cli_cancel_secret();

update_option( 'boldgrid_backup_pending_rollback', 'test' );

Expand All @@ -84,18 +127,56 @@ public function test_wp_ajax_cli_cancel_failure() {
echo null;
}

/*
* This option was given a value above, and because the cancel rollback method failed, the
* value should still be the same.
*/
$this->assertEquals( 'test', get_option( 'boldgrid_backup_pending_rollback' ) );
$this->assertInvalidArgumentsResponse();
}

$response = json_decode( $this->_last_response );
/**
* Test wp_ajax_cli_cancel fails when cli_cancel_secret is missing.
*
* This covers the reported vulnerability: an attacker who can compute or guess
* the backup_id must not be able to cancel a rollback without also knowing the
* one-time random secret stored server-side.
*
* @since 1.17.2
*/
public function test_wp_ajax_cli_cancel_no_secret() {
global $_GET; // phpcs:ignore
$_GET['backup_id'] = $this->core->get_backup_identifier();
unset( $_GET['cli_cancel_secret'] );

$this->assertInternalType( 'object', $response ); // Should be an object.
$this->assertObjectHasAttribute( 'success', $response ); // Should have "success".
$this->assertFalse( $response->success ); // "success" should be FALSE.
$this->assertObjectHasAttribute( 'data', $response ); // Should have "data".
$this->assertEquals( 'Invalid arguments', $response->data ); // "data" is a string; must match expected string.
$this->set_cli_cancel_secret(); // Secret is stored but not provided in the request.
update_option( 'boldgrid_backup_pending_rollback', 'test' );

try {
$this->_handleAjax( 'nopriv_boldgrid_cli_cancel_rollback' );
} catch ( WPAjaxDieContinueException $e ) {
echo null;
}

$this->assertInvalidArgumentsResponse();
}

/**
* Test wp_ajax_cli_cancel fails when an incorrect cli_cancel_secret is provided.
*
* Even with a valid backup_id, supplying the wrong secret must be rejected.
*
* @since 1.17.2
*/
public function test_wp_ajax_cli_cancel_wrong_secret() {
global $_GET; // phpcs:ignore
$_GET['backup_id'] = $this->core->get_backup_identifier();
$_GET['cli_cancel_secret'] = 'this_is_not_the_correct_secret';

$this->set_cli_cancel_secret( 'correct_secret_stored_server_side' ); // Stored secret differs.
update_option( 'boldgrid_backup_pending_rollback', 'test' );

try {
$this->_handleAjax( 'nopriv_boldgrid_cli_cancel_rollback' );
} catch ( WPAjaxDieContinueException $e ) {
echo null;
}

$this->assertInvalidArgumentsResponse();
}
}
78 changes: 78 additions & 0 deletions tests/admin/test-class-boldgrid-backup-admin-cron.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,84 @@ public function set_up() {
';
}

/**
* Ensure at least one backup archive exists on disk so that get_restore_command()
* can successfully initialize an archive via init_by_key(0).
*
* @since 1.17.2
*/
private function maybe_create_backup() {
$latest_backup = get_option( 'boldgrid_backup_latest_backup' );
if ( empty( $latest_backup ) ) {
$this->core->archive_files( true );
}
}

/**
* Test that get_restore_command() generates a secret, stores it in the site option,
* and embeds the same value in the returned cron entry string.
*
* @since 1.17.2
*/
public function test_get_restore_command_stores_secret() {
$this->maybe_create_backup();

delete_site_option( 'boldgrid_backup_cli_cancel_secret' );

$entry = $this->core->cron->get_restore_command();

$this->assertNotEmpty( $entry );

$stored_secret = get_site_option( 'boldgrid_backup_cli_cancel_secret', false );
$this->assertNotFalse( $stored_secret );
$this->assertNotEmpty( $stored_secret );

$this->assertStringContainsString( 'cli_cancel_secret=' . $stored_secret, $entry );
}

/**
* Test that each call to get_restore_command() generates a fresh secret.
*
* The previous secret must not be reusable after a new restore is scheduled.
*
* @since 1.17.2
*/
public function test_get_restore_command_secret_rotates() {
$this->maybe_create_backup();

$this->core->cron->get_restore_command();
$first_secret = get_site_option( 'boldgrid_backup_cli_cancel_secret', false );

$this->core->cron->get_restore_command();
$second_secret = get_site_option( 'boldgrid_backup_cli_cancel_secret', false );

$this->assertNotFalse( $first_secret );
$this->assertNotFalse( $second_secret );
$this->assertNotEquals( $first_secret, $second_secret );
}

/**
* Test that get_restore_command() returns an empty string and does not store a
* secret when no backup archive is available.
*
* @since 1.17.2
*/
public function test_get_restore_command_no_archive() {
// Redirect the backup dir to a non-existent path so get_archive_list() returns [].
$original_dir = $this->core->backup_dir->backup_directory;
$this->core->backup_dir->backup_directory = '/nonexistent/path/that/does/not/exist';

delete_site_option( 'boldgrid_backup_cli_cancel_secret' );

$entry = $this->core->cron->get_restore_command();

// Restore the backup dir before any assertions that could fail.
$this->core->backup_dir->backup_directory = $original_dir;

$this->assertEmpty( $entry );
$this->assertFalse( get_site_option( 'boldgrid_backup_cli_cancel_secret', false ) );
}

/**
* Test filtering crontab contents with mode "" (backup).
*
Expand Down