1.7.1: security fixes for the checkout and the client IP
CI / WordPress latest + WooCommerce latest on PHP 8.3 (pull_request) Successful in 1m14s
Compatibility / WP latest / WC latest / PHP 7.4 (pull_request) Skipped
Compatibility / WP latest / WC beta / PHP 8.3 (pull_request) Skipped
Compatibility / WP latest / WC latest / PHP 8.3 (pull_request) Skipped
Compatibility / WP nightly / WC latest / PHP 8.3 (pull_request) Skipped
Compatibility / WP latest / WC latest / PHP 8.4 (pull_request) Skipped
Compatibility / Keep the compatibility headers honest (pull_request) Skipped
CI / Static checks (pull_request) Successful in 4m34s
CI / WordPress latest + WooCommerce latest on PHP 8.3 (pull_request) Successful in 1m14s
Compatibility / WP latest / WC latest / PHP 7.4 (pull_request) Skipped
Compatibility / WP latest / WC beta / PHP 8.3 (pull_request) Skipped
Compatibility / WP latest / WC latest / PHP 8.3 (pull_request) Skipped
Compatibility / WP nightly / WC latest / PHP 8.3 (pull_request) Skipped
Compatibility / WP latest / WC latest / PHP 8.4 (pull_request) Skipped
Compatibility / Keep the compatibility headers honest (pull_request) Skipped
CI / Static checks (pull_request) Successful in 4m34s
- The discount follows the billing email, the address the order is placed under. The tracked session email used to win, so a guest could post a discounted address to the tracking endpoint and check out under their own, keeping the discount. - Shipping packages carry the matched assignment, so WooCommerce's per-package rate cache no longer reuses rates filtered for a matched address (free shipping, reserved methods) after the email changes. - The anonymous tracking endpoint no longer answers matched: true|false, which let anyone enumerate discounted addresses and domains. - CF-Connecting-IP is believed only from a Cloudflare edge range or a private/loopback proxy. From anyone else it let a direct client pick a new address per request: no REST rate limit, forged audit-log IPs. Smoke tests: 89 assertions (was 73), each fix with its own. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
4a93a5fde8
commit
d5d85612bd
@@ -318,6 +318,92 @@ WC()->customer->set_billing_email( '[email protected]' );
|
||||
|
||||
WC_EDM_DB::replace_shipping_rules( (int) $fixtures['group_id'], array() );
|
||||
|
||||
/* -------------------------------------------------------------- security -- */
|
||||
|
||||
// 🔴 The discount follows the address the order is placed under. Up to 1.7.0
|
||||
// the tracked session email won over the billing email, so a visitor could
|
||||
// post a discounted address to the tracking endpoint and then check out under
|
||||
// their own, taking the discount with nothing on the order pointing at it.
|
||||
wc_edm_cart_for( '[email protected]', array( $fixtures['product_in'] => 1, $fixtures['product_out'] => 1 ) );
|
||||
WC()->session->set( WC_EDM_Frontend::SESSION_EMAIL_KEY, '[email protected]' );
|
||||
WC()->cart->calculate_totals();
|
||||
wc_edm_is( 'a tracked address does not override the billing email', '[email protected]', wc_edm()->frontend->get_customer_email() );
|
||||
wc_edm_is( 'and buys no discount for an order placed under another address', 0.0, wc_edm_discount_total() );
|
||||
|
||||
WC()->customer->set_billing_email( '' );
|
||||
wc_edm_is( 'the tracked address is still used while there is no billing email', '[email protected]', wc_edm()->frontend->get_customer_email() );
|
||||
WC()->session->set( WC_EDM_Frontend::SESSION_EMAIL_KEY, null );
|
||||
|
||||
// 🔴 WooCommerce caches a package's rates under a hash of the package, which
|
||||
// did not include who the customer is — so rates filtered for a matched
|
||||
// address (free shipping, a reserved method) survived a change of email.
|
||||
WC()->customer->set_billing_email( '[email protected]' );
|
||||
$wc_edm_pkg_a = wc_edm()->frontend->key_packages_to_match( array( array( 'contents' => array() ) ) );
|
||||
WC()->customer->set_billing_email( '[email protected]' );
|
||||
$wc_edm_pkg_b = wc_edm()->frontend->key_packages_to_match( array( array( 'contents' => array() ) ) );
|
||||
wc_edm_ok(
|
||||
'a shipping package carries the match, so cached rates do not outlive a change of email',
|
||||
$wc_edm_pkg_a[0]['wc_edm_match'] > 0 && 0 === $wc_edm_pkg_b[0]['wc_edm_match'],
|
||||
sprintf( 'matched %s, unmatched %s', var_export( $wc_edm_pkg_a[0]['wc_edm_match'], true ), var_export( $wc_edm_pkg_b[0]['wc_edm_match'], true ) )
|
||||
);
|
||||
|
||||
// 🔴 The tracking endpoint answers anonymous visitors. It used to return
|
||||
// `matched: true|false`, which told a stranger which addresses and domains the
|
||||
// shop discounts, one request per guess. The answer must be the same bare
|
||||
// success for a matched and an unmatched address.
|
||||
add_filter( 'wp_doing_ajax', '__return_true' );
|
||||
$wc_edm_die = static function () {
|
||||
return static function () {
|
||||
throw new RuntimeException( 'wp_die' );
|
||||
};
|
||||
};
|
||||
add_filter( 'wp_die_ajax_handler', $wc_edm_die );
|
||||
$wc_edm_track = static function ( $email ) {
|
||||
$_POST['email'] = $email;
|
||||
$_REQUEST['nonce'] = wp_create_nonce( 'wc-edm-track-email' );
|
||||
ob_start();
|
||||
try {
|
||||
wc_edm()->frontend->ajax_track_email();
|
||||
} catch ( RuntimeException $e ) {
|
||||
unset( $e );
|
||||
}
|
||||
return trim( (string) ob_get_clean() );
|
||||
};
|
||||
$wc_edm_said_match = $wc_edm_track( '[email protected]' );
|
||||
wc_edm_is( 'tracking a matched address sets it as the billing email', '[email protected]', WC()->customer->get_billing_email() );
|
||||
$wc_edm_said_nomatch = $wc_edm_track( '[email protected]' );
|
||||
wc_edm_is( 'the tracking endpoint answers a matched address with a bare success', '{"success":true}', $wc_edm_said_match );
|
||||
wc_edm_is( 'and an unmatched one identically', $wc_edm_said_match, $wc_edm_said_nomatch );
|
||||
remove_filter( 'wp_die_ajax_handler', $wc_edm_die );
|
||||
remove_filter( 'wp_doing_ajax', '__return_true' );
|
||||
unset( $_POST['email'], $_REQUEST['nonce'] );
|
||||
WC()->session->set( WC_EDM_Frontend::SESSION_EMAIL_KEY, null );
|
||||
WC()->customer->set_billing_email( '[email protected]' );
|
||||
|
||||
// 🔴 CF-Connecting-IP is believed only from a proxy. Up to 1.7.0 it was
|
||||
// believed from anyone, so a direct client chose a new address per request:
|
||||
// no REST rate limit, and an audit log recording whatever it was told.
|
||||
$wc_edm_server = $_SERVER;
|
||||
$wc_edm_ip = static function ( $remote, $header ) {
|
||||
$_SERVER['REMOTE_ADDR'] = $remote;
|
||||
if ( null === $header ) {
|
||||
unset( $_SERVER['HTTP_CF_CONNECTING_IP'] );
|
||||
} else {
|
||||
$_SERVER['HTTP_CF_CONNECTING_IP'] = $header;
|
||||
}
|
||||
return WC_EDM_Audit_Log::get_client_ip();
|
||||
};
|
||||
wc_edm_is( 'a direct client cannot choose its address with CF-Connecting-IP', '203.0.113.7', $wc_edm_ip( '203.0.113.7', '198.51.100.1' ) );
|
||||
wc_edm_is( 'a request from a Cloudflare edge is attributed to the visitor', '198.51.100.1', $wc_edm_ip( '162.158.1.2', '198.51.100.1' ) );
|
||||
wc_edm_is( 'and so is one from an IPv6 edge', '198.51.100.1', $wc_edm_ip( '2606:4700::1', '198.51.100.1' ) );
|
||||
wc_edm_is( 'and one from a local tunnel', '198.51.100.1', $wc_edm_ip( '127.0.0.1', '198.51.100.1' ) );
|
||||
wc_edm_is( 'and one from a proxy on the private network', '198.51.100.1', $wc_edm_ip( '172.18.0.3', '198.51.100.1' ) );
|
||||
wc_edm_is( 'an address just outside a Cloudflare range is not a proxy', '173.245.64.1', $wc_edm_ip( '173.245.64.1', '198.51.100.1' ) );
|
||||
wc_edm_is( 'and the last one inside it is', '198.51.100.1', $wc_edm_ip( '173.245.63.255', '198.51.100.1' ) );
|
||||
wc_edm_is( 'a malformed header from a proxy falls back to the proxy', '162.158.1.2', $wc_edm_ip( '162.158.1.2', 'not-an-ip' ) );
|
||||
wc_edm_is( 'no header means REMOTE_ADDR', '203.0.113.7', $wc_edm_ip( '203.0.113.7', null ) );
|
||||
$_SERVER = $wc_edm_server;
|
||||
|
||||
/* ------------------------------------------------------------------ rest -- */
|
||||
|
||||
$routes = rest_get_server()->get_routes();
|
||||
|
||||
Reference in New Issue
Block a user