Skip to content

Fix unauthenticated rollback cancellation#633

Merged
cssjoe merged 8 commits intomasterfrom
ENG7-1452-access-vul
Mar 11, 2026
Merged

Fix unauthenticated rollback cancellation#633
cssjoe merged 8 commits intomasterfrom
ENG7-1452-access-vul

Conversation

@jacobd91
Copy link
Contributor

@jacobd91 jacobd91 commented Mar 2, 2026

https://imh-internal.atlassian.net/browse/ENG7-1452

Vulnerability

The boldgrid_cli_cancel_rollback nopriv AJAX endpoint was guarded only by a static backup_id. This identifier is a CRC32 hash
of the site URL and admin email:

hash( 'crc32', hash( 'sha512', site_url() . ' <' . $admin_email . '>' ) )

Because CRC32 produces only ~4 billion possible values, and the inputs are often discoverable (the site URL is public; the
admin email is frequently exposed via the WP REST API /wp-json/wp/v2/users/1 or author slugs), an unauthenticated attacker
could cancel a pending auto-rollback — silently preventing the safety net from reverting a broken plugin update.

The endpoint must remain nopriv because the CLI cron process has no WordPress session and cannot generate a nonce.

Replicating the vulnerability (before patch)

Prerequisites: BoldGrid Backup installed, auto-rollback enabled, at least one backup on disk.

  1. Trigger a pending rollback — install or activate any plugin while auto-rollback is enabled. This schedules a system cron
    job to restore the last backup in ~15 minutes.
  2. Determine the backup_id via any of:
    - Reading wp_sitemeta where meta_key = 'boldgrid_backup_id'
    - Computing hash('crc32', hash('sha512', 'https://example.com admin@example.com')) from the public site URL and
    discoverable admin email
    - Brute-forcing the ~4 billion CRC32 values
  3. Send an unauthenticated request:
    curl "https://example.com/wp-admin/admin-ajax.php?action=boldgrid_cli_cancel_rollback&backup_id=<backup_id>"
  4. Result: {"success":true,"data":"Rollback canceled"} — the auto-rollback is silently aborted with no authentication.

Fix

A cryptographically random 32-character secret is generated with wp_generate_password(32, false) each time a restore cron job
is scheduled. It is:

  • Stored server-side as a site option (boldgrid_backup_cli_cancel_secret)
  • Passed as a CLI argument to the cron process (cli_cancel_secret=)
  • Required alongside backup_id on the cancel endpoint, validated with hash_equals()
  • Deleted from the database after a successful cancel (one-time use)

The CLI script already receives the secret in $argv and forwards it in the cancel URL automatically — no change in observable
behavior for legitimate usage. This mirrors the pattern already used by the boldgrid_backup_run_backup and
boldgrid_backup_run_restore nopriv endpoints.

Testing the patch

Run the automated test suite:
./vendor/bin/phpunit
Expected: 95 tests, 324 assertions, 0 failures.

Key tests covering this fix:

┌─────────────────────────────────────────┬───────────────────────────────────────────────────────────────────────────────┐
│                  Test                   │                               What it verifies                                │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_wp_ajax_cli_cancel_success         │ Valid backup_id + valid secret → cancel succeeds, secret deleted              │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_wp_ajax_cli_cancel_failure         │ Missing backup_id → rejected                                                  │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_wp_ajax_cli_cancel_no_secret       │ Valid backup_id, secret omitted → rejected                                    │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_wp_ajax_cli_cancel_wrong_secret    │ Valid backup_id, wrong secret → rejected                                      │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_get_restore_command_stores_secret  │ Scheduling a restore generates and stores the secret, embedded in the cron    │
│                                         │ entry                                                                         │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_get_restore_command_secret_rotates │ Each new schedule produces a different secret                                 │
├─────────────────────────────────────────┼───────────────────────────────────────────────────────────────────────────────┤
│ test_get_restore_command_no_archive     │ No archive → no secret stored, empty entry returned                           │
└─────────────────────────────────────────┴───────────────────────────────────────────────────────────────────────────────┘

Manual verification (patched): Repeat the attack request without the secret:
curl "https://example.com/wp-admin/admin-ajax.php?action=boldgrid_cli_cancel_rollback&backup_id=<backup_id>"
Expected result: {"success":false,"data":"Invalid arguments"} — request is rejected. The legitimate cron-driven cancel flow
continues to work without any change.

…ecret

The nopriv boldgrid_cli_cancel_rollback AJAX endpoint was guarded only by
a static backup_id (a CRC32 hash derived from site URL + admin email),
allowing an unauthenticated attacker to cancel a pending auto-rollback by
computing or brute-forcing the identifier.

Fix generates a cryptographically random 32-character secret at restore-cron
schedule time, stores it server-side, passes it as a CLI argument to the cron
job, and requires it alongside backup_id on the cancel endpoint. The secret is
deleted after a successful cancel so it cannot be replayed.

Also upgrades PHPUnit from ^7 to ^9 and adds three new cron tests covering
secret generation, rotation, and the no-archive early-exit path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the unauthenticated boldgrid_cli_cancel_rollback AJAX endpoint so pending auto-rollbacks cannot be canceled without an additional one-time secret, while keeping the endpoint usable by the CLI cron process.

Changes:

  • Generate and store a cryptographically random one-time secret when scheduling a restore cron, and include it in the cron CLI args.
  • Require the secret (in addition to backup_id) on the nopriv cancel endpoint, using hash_equals(), and delete the secret after a successful cancel.
  • Add/extend PHPUnit coverage for restore command secret generation/rotation and for cancel endpoint success/failure scenarios.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
admin/class-boldgrid-backup-admin-cron.php Generates and persists a one-time cancel secret and embeds it into the restore cron entry.
admin/class-boldgrid-backup-admin-auto-rollback.php Enforces secret validation for CLI cancel AJAX and deletes the secret upon successful cancel.
cli/class-site-restore.php Appends the new secret argument to the cancel URL (with URL encoding).
tests/admin/test-class-boldgrid-backup-admin-cron.php Adds tests ensuring the restore command stores/rotates the secret and doesn’t store it when no archive exists.
tests/admin/test-class-boldgrid-backup-admin-ajax.php Expands AJAX tests to require the secret and validate failure modes (missing/wrong secret).
composer.json Updates dev dependency constraint for PHPUnit to ^9.
composer.lock Updates locked dev dependency set (notably PHPUnit and related packages).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jacobd91 and others added 7 commits March 2, 2026 19:58
PHPUnit 9.6.34 allows doctrine/instantiator ^1.5.0 || ^2. Composer was
resolving to 2.1.0, which requires PHP ^8.4, breaking CI builds on
PHP 7.4 and 8.0. Pinning to ^1.5 keeps the dependency compatible with
both environments while PHPUnit 9 continues to work locally and in CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The composer self-update --1 and composer remove/require cycle were
designed to let Composer 1 pick the best phpunit version per PHP
environment. Packagist shut down Composer 1 API support in September
2025, making composer require fail with "Could not find package".

Now that composer.json constrains phpunit to ^9 with doctrine/instantiator
pinned to ^1.5 (PHP 7.4/8.0 compatible) and composer.lock is committed,
composer install -o is sufficient and correct for all CI environments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
get_arg_value() returns null when a CLI arg is absent. Passing null to
rawurlencode() throws a TypeError on PHP 8+, which would fatal an
already-scheduled pre-secret cron entry running after the plugin updates.
Casting to string yields an empty string, letting the endpoint reject
the request gracefully via its existing !empty() checks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@cssjoe
Copy link
Member

cssjoe commented Mar 11, 2026

Warning from https://repo.packagist.org: Support for Composer 1 has been shutdown on September 1st 2025. You should upgrade to Composer 2. See https://blog.packagist.com/shutting-down-packagist-org-support-for-composer-1-x/

@cssjoe cssjoe merged commit 0e7c604 into master Mar 11, 2026
1 of 3 checks passed
@cssjoe cssjoe deleted the ENG7-1452-access-vul branch March 11, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants