fix/WPS-7849-vulnerability-issue-fix

includes/membership/class-wps-restriction-enforcer.php

@@ -110,9 +110,17 @@ public function maybe_restrict_content( $content ) {
 					: get_option( 'wps_access_redirect_url', '' );
 
 				if ( ! empty( $url ) ) {
-					// maybe_redirect() has already called wp_safe_redirect() + exit.
-					// We only reach here in edge cases (e.g. unit tests); pass through.
-					return $content;
+					// maybe_redirect() fires on template_redirect for singular
+					// front-end views and will have already exited before the
+					// content filter runs. In REST API and feed contexts,
+					// template_redirect never runs, so we must show the restriction
+					// message here rather than returning the real content.
+					$is_rest = defined( 'REST_REQUEST' ) && REST_REQUEST;
+					if ( ! $is_rest && ! is_feed() ) {
+						// Standard singular request — redirect already fired.
+						return $content;
+					}
+					// REST API / feed: fall through to the restriction message.
 				}
 				// No URL configured — fall through and show message instead.
 			}
@@ -408,8 +416,9 @@ public function maybe_close_comments( $open, $post_id ) {
 		/**
 		 * Exclude restricted posts from archive and search queries.
 		 *
-		 * Fires on pre_get_posts (wired in register_shortcode). Only modifies the
-		 * main query outside admin/singular views. Respects the global
+		 * Fires on pre_get_posts (wired in register_shortcode). Modifies the main
+		 * query outside admin/singular views, and also REST API collection queries
+		 * which are not the WP global main query. Respects the global
 		 * wps_access_include_in_archive option — when '1', restricted content is
 		 * shown in archives (but content is replaced on the singular view).
 		 *
@@ -421,7 +430,17 @@ public function maybe_close_comments( $open, $post_id ) {
 		 * @param  WP_Query $query The current WP_Query object.
 		 */
 		public function maybe_filter_archive( WP_Query $query ) {
-			if ( is_admin() || ! $query->is_main_query() || is_singular() ) {
+			$is_rest = defined( 'REST_REQUEST' ) && REST_REQUEST;
+
+			if ( is_admin() || is_singular() ) {
+				return;
+			}
+
+			// For standard front-end requests, only the main query is modified so
+			// that secondary loops (e.g. related-posts widgets) are unaffected.
+			// REST API collection queries are not the WP global main query but must
+			// also exclude restricted posts, so we allow them through separately.
+			if ( ! $is_rest && ! $query->is_main_query() ) {
 				return;
 			}
 

public/class-subscriptions-for-woocommerce-public.php

@@ -1346,6 +1346,13 @@ public function wps_sfw_cancel_susbcription() {
 
 		$user_id = get_current_user_id();
 
+		// Confirm the current user owns this subscription before acting on it.
+		// This reuses the same ownership helper used by the view path a few lines
+		// above, so the cancel and view paths enforce exactly the same rule.
+		if ( ! $this->wps_sfw_current_user_can_view_subscription( $wps_subscription_id ) ) {
+			return;
+		}
+
 		if ( wps_sfw_check_valid_subscription( $wps_subscription_id ) ) {
 			$this->wps_sfw_cancel_susbcription_order_by_customer( $wps_subscription_id, $wps_status, $user_id );
 		}
@@ -1868,6 +1875,7 @@ public function wps_sfw_calculate_recurring_price( $cart_item, $bool ) {
 
 		$line_subtotal_tax = 0;
 		$line_tax = 0;
+		$wps_sfw_price_overridden = false;
 
 		// Get the only item price from the cart.
 		if ( 'yes' === $include_tax ) {
@@ -1910,6 +1918,7 @@ public function wps_sfw_calculate_recurring_price( $cart_item, $bool ) {
 			$price         = $product->get_price() * $cart_item['quantity'];
 			$line_subtotal = $price;
 			$line_total    = $price;
+			$wps_sfw_price_overridden = true;
 
 			$get_membershipprice = wps_sfw_get_meta_data( $product_id, 'wps_membership_plan_price', true );
 			if ( ! empty( $get_membershipprice ) ) {
@@ -1924,17 +1933,36 @@ public function wps_sfw_calculate_recurring_price( $cart_item, $bool ) {
 
 		$product   = $cart_item['data'];
 		$tax_class = $product->get_tax_class();
-		$tax_rates = WC_Tax::get_rates( $tax_class );
 		if ( function_exists( 'wps_sfw_is_woocommerce_tax_enabled' ) && wps_sfw_is_woocommerce_tax_enabled() ) {
-			if ( 'yes' === $include_tax ) {
-				$line_subtotal_tax = WC_Tax::get_tax_total( WC_Tax::calc_inclusive_tax( $line_subtotal, $tax_rates ) );
-				$line_tax     = WC_Tax::get_tax_total( WC_Tax::calc_inclusive_tax( $line_total, $tax_rates ) );
-
-				$line_total    = $line_total - $line_tax;
-				$line_subtotal = $line_subtotal - $line_subtotal_tax;
+			if ( $wps_sfw_price_overridden ) {
+				// The free trial / membership price above replaced the cart's own line totals,
+				// so the cart's tax split (below) no longer corresponds to this amount - it has
+				// to be re-derived for this specific overridden price.
+				$tax_rates = WC_Tax::get_rates( $tax_class );
+				if ( 'yes' === $include_tax ) {
+					$line_subtotal_tax = WC_Tax::get_tax_total( WC_Tax::calc_inclusive_tax( $line_subtotal, $tax_rates ) );
+					$line_tax     = WC_Tax::get_tax_total( WC_Tax::calc_inclusive_tax( $line_total, $tax_rates ) );
+
+					$line_total    = $line_total - $line_tax;
+					$line_subtotal = $line_subtotal - $line_subtotal_tax;
+				} else {
+					$line_subtotal_tax = WC_Tax::get_tax_total( WC_Tax::calc_exclusive_tax( $line_subtotal, $tax_rates ) );
+					$line_tax     = WC_Tax::get_tax_total( WC_Tax::calc_exclusive_tax( $line_total, $tax_rates ) );
+				}
 			} else {
-				$line_subtotal_tax = WC_Tax::get_tax_total( WC_Tax::calc_exclusive_tax( $line_subtotal, $tax_rates ) );
-				$line_tax     = WC_Tax::get_tax_total( WC_Tax::calc_exclusive_tax( $line_total, $tax_rates ) );
+				// Reuse the tax WooCommerce's own cart calculation already derived for this
+				// specific customer (correct destination tax rate, currency, and composite/bundle
+				// pricing) instead of re-deriving it via WC_Tax::get_rates(), which resolves to
+				// the product's default tax-class rate rather than the rate actually owed by this
+				// customer. That mismatch silently double-deducts tax whenever the two rates
+				// differ - e.g. a non-EU customer whose real rate is 0% on a store where prices
+				// are entered VAT-inclusive ends up having ~20% deducted a second time here.
+				$line_subtotal_tax = $cart_item['line_subtotal_tax'];
+				$line_tax           = $cart_item['line_tax'];
+				if ( 'yes' === $include_tax ) {
+					$line_total    = $line_total - $line_tax;
+					$line_subtotal = $line_subtotal - $line_subtotal_tax;
+				}
 			}
 		}
 

tests/Unit/CancelSubscriptionTest.php

@@ -0,0 +1,227 @@
+<?php
+/**
+ * Unit tests for CVE-2: subscription cancellation via CSRF / missing ownership check.
+ *
+ * The handler wps_sfw_cancel_susbcription() runs on `init` and previously:
+ *   1. Accepted any request where _wpnonce was merely present (token not verified).
+ *   2. Never called the ownership helper, relying only on the inner cancel
+ *      function's loose `$wps_customer_id == $user_id` check.
+ *
+ * After the fix:
+ *   1. wp_verify_nonce() is called; a token that is merely present but invalid
+ *      is rejected.
+ *   2. wps_sfw_current_user_can_view_subscription() gates the cancel path so
+ *      a user who does not own the subscription cannot cancel it, even with a
+ *      cryptographically valid nonce for the right action.
+ *
+ * @since   2.0.2
+ * @package Subscriptions_For_Woocommerce
+ */
+
+/**
+ * Tests for the customer-facing subscription cancellation handler.
+ */
+class CancelSubscriptionTest extends WP_UnitTestCase {
+
+	/**
+	 * The public class under test.
+	 *
+	 * @var Subscriptions_For_Woocommerce_Public
+	 */
+	private $public_obj;
+
+	/**
+	 * Post ID of the fake subscription owned by $this->owner_id.
+	 *
+	 * @var int
+	 */
+	private $subscription_id;
+
+	/**
+	 * User ID of the subscription owner.
+	 *
+	 * @var int
+	 */
+	private $owner_id;
+
+	/**
+	 * User ID of a different logged-in user who does not own the subscription.
+	 *
+	 * @var int
+	 */
+	private $stranger_id;
+
+	/**
+	 * Set up fixtures before each test.
+	 */
+	public function setUp(): void {
+		parent::setUp();
+
+		require_once SUBSCRIPTIONS_FOR_WOOCOMMERCE_DIR_PATH
+			. 'public/class-subscriptions-for-woocommerce-public.php';
+
+		$this->public_obj = new Subscriptions_For_Woocommerce_Public(
+			'subscriptions-for-woocommerce',
+			SUBSCRIPTIONS_FOR_WOOCOMMERCE_VERSION
+		);
+
+		$this->owner_id   = $this->factory->user->create( array( 'role' => 'subscriber' ) );
+		$this->stranger_id = $this->factory->user->create( array( 'role' => 'subscriber' ) );
+
+		// Create a minimal subscription post so wps_sfw_check_valid_subscription() passes.
+		$this->subscription_id = $this->factory->post->create(
+			array(
+				'post_type'   => 'wps_subscriptions',
+				'post_status' => 'publish',
+			)
+		);
+
+		// Tag the subscription with the owner's user ID (mirrors what the plugin writes).
+		wps_sfw_update_meta_data( $this->subscription_id, 'wps_customer_id', $this->owner_id );
+		wps_sfw_update_meta_data( $this->subscription_id, 'wps_subscription_status', 'active' );
+	}
+
+	/**
+	 * Tear down fixtures after each test.
+	 */
+	public function tearDown(): void {
+		unset( $_GET['wps_subscription_id'], $_GET['wps_subscription_status'], $_GET['_wpnonce'] );
+		wp_delete_post( $this->subscription_id, true );
+		wp_delete_user( $this->owner_id );
+		wp_delete_user( $this->stranger_id );
+		parent::tearDown();
+	}
+
+	// -------------------------------------------------------------------------
+	// Helpers
+	// -------------------------------------------------------------------------
+
+	/**
+	 * Populate $_GET with the cancel parameters including a genuine WP nonce
+	 * created for the current user's session.
+	 *
+	 * @param int    $subscription_id Subscription post ID.
+	 * @param string $status          wps_subscription_status value.
+	 * @param string $nonce_override  Pass a custom nonce string to simulate an invalid token.
+	 */
+	private function set_cancel_request( $subscription_id, $status = 'active', $nonce_override = null ) {
+		$_GET['wps_subscription_id']     = (string) $subscription_id;
+		$_GET['wps_subscription_status'] = $status;
+		$_GET['_wpnonce']                = $nonce_override ?? wp_create_nonce( $subscription_id . $status );
+	}
+
+	/**
+	 * Return the stored subscription status directly from meta.
+	 *
+	 * @param int $subscription_id Subscription post ID.
+	 * @return string
+	 */
+	private function get_status( $subscription_id ) {
+		return (string) wps_sfw_get_meta_data( $subscription_id, 'wps_subscription_status', true );
+	}
+
+	// -------------------------------------------------------------------------
+	// Missing parameters
+	// -------------------------------------------------------------------------
+
+	/** Handler returns early when _wpnonce is absent. */
+	public function test_missing_nonce_is_rejected() {
+		wp_set_current_user( $this->owner_id );
+		$_GET['wps_subscription_id']     = (string) $this->subscription_id;
+		$_GET['wps_subscription_status'] = 'active';
+		// Deliberately omit _wpnonce.
+
+		$this->public_obj->wps_sfw_cancel_susbcription();
+
+		$this->assertSame( 'active', $this->get_status( $this->subscription_id ) );
+	}
+
+	// -------------------------------------------------------------------------
+	// Nonce verification (CVE-2 part 1)
+	// -------------------------------------------------------------------------
+
+	/** A token that is merely present but cryptographically invalid is rejected. */
+	public function test_invalid_nonce_is_rejected() {
+		wp_set_current_user( $this->owner_id );
+		$this->set_cancel_request( $this->subscription_id, 'active', 'x' );
+
+		$this->public_obj->wps_sfw_cancel_susbcription();
+
+		$this->assertSame( 'active', $this->get_status( $this->subscription_id ) );
+	}
+
+	/** Whitespace-only nonce is rejected. */
+	public function test_whitespace_nonce_is_rejected() {
+		wp_set_current_user( $this->owner_id );
+		$this->set_cancel_request( $this->subscription_id, 'active', '   ' );
+
+		$this->public_obj->wps_sfw_cancel_susbcription();
+
+		$this->assertSame( 'active', $this->get_status( $this->subscription_id ) );
+	}
+
+	// -------------------------------------------------------------------------
+	// Ownership check (CVE-2 part 2)
+	// -------------------------------------------------------------------------
+
+	/**
+	 * A logged-in user with a valid nonce cannot cancel a subscription they
+	 * do not own.
+	 *
+	 * Before the fix the handler skipped the ownership check entirely; the
+	 * inner function's loose equality check was the only barrier, and in some
+	 * edge cases (e.g. empty stored customer ID) it could be bypassed.
+	 */
+	public function test_non_owner_cannot_cancel_with_valid_nonce() {
+		wp_set_current_user( $this->stranger_id );
+		// Create a nonce for the stranger's session — it is cryptographically
+		// valid for this user, but they do not own the subscription.
+		$this->set_cancel_request( $this->subscription_id );
+
+		$this->public_obj->wps_sfw_cancel_susbcription();
+
+		$this->assertSame(
+			'active',
+			$this->get_status( $this->subscription_id ),
+			'A non-owner must not be able to cancel a subscription they do not own'
+		);
+	}
+
+	/** Guest (user ID 0) cannot cancel even with a parameter set. */
+	public function test_guest_cannot_cancel() {
+		wp_set_current_user( 0 );
+		$this->set_cancel_request( $this->subscription_id, 'active', 'anything' );
+
+		$this->public_obj->wps_sfw_cancel_susbcription();
+
+		$this->assertSame( 'active', $this->get_status( $this->subscription_id ) );
+	}
+
+	// -------------------------------------------------------------------------
+	// Happy path
+	// -------------------------------------------------------------------------
+
+	/**
+	 * Owner with a valid nonce and an 'active' subscription status can cancel.
+	 *
+	 * We stub wp_safe_redirect/exit via output buffering; the test catches the
+	 * redirect exception (some test suites wrap wp_safe_redirect + exit to
+	 * throw WPDieException) or simply checks the meta after the call.
+	 */
+	public function test_owner_can_cancel_with_valid_nonce() {
+		wp_set_current_user( $this->owner_id );
+		$this->set_cancel_request( $this->subscription_id );
+
+		try {
+			$this->public_obj->wps_sfw_cancel_susbcription();
+		} catch ( WPDieException $e ) {
+			// wp_safe_redirect + exit was called — that is the expected path.
+		}
+
+		$this->assertSame(
+			'cancelled',
+			$this->get_status( $this->subscription_id ),
+			'The owner with a valid nonce must be able to cancel their own subscription'
+		);
+	}
+}