Skip to content

Commit f8131f0

Browse files
deepenchMILAN88888
andauthored
#1560 Fix - Membership thank you page leaking another member's account details (#1427)
* #1408 Fix - Prevent membership privilege escalation and open redirect after login * #1408 Fix - Address Copilot review: document meta auth_callback signature and drop dead AJAX-login code * #1408 Fix - Match the meta auth_callback to WP's 6-arg filter signature and dedupe the redirect host * #1408 Fix - Evaluate the meta auth_callback against the user WordPress passes, not the current user * #1408 Fix - Reject a forged gateway on a free membership so it cannot take the paid order path * #1560 Fix - Prevent membership thank you page leaking another member's account details * #1560 Fix - Address Copilot review: defer_role by plan type, sanitize transaction_id, respect meta auth chain --------- Co-authored-by: milan88888 <chaudharymilan996@gmail.com>
1 parent e537e78 commit f8131f0

4 files changed

Lines changed: 26 additions & 15 deletions

File tree

modules/membership/includes/Admin/Services/MembersService.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ public function prepare_members_data( $data, $context = 'admin' ) {
163163
'membership' => absint( $data['membership'] ),
164164
'start_date' => date( 'Y-m-d', strtotime( $data['start_date'] ) ),
165165
'payment_method' => sanitize_text_field( $data['payment_method'] ?? '' ),
166+
'type' => isset( $membership_meta['type'] ) ? sanitize_text_field( $membership_meta['type'] ) : 'unknown',
166167
);
167168

168169
if ( isset( $data['is_purchasing_multiple'] ) ) {

modules/membership/includes/Admin/Services/MembershipService.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,9 @@ public function create_membership_order_and_subscription( $data ) {
233233
$members_data = $this->members_service->prepare_members_data( $data, 'frontend' );
234234
$member = get_user_by( 'login', $data['username'] );
235235

236-
// Update user source and add membership_role.
237-
$members_data['defer_role'] = ! empty( $members_data['membership_data']['payment_method'] ) && 'free' !== $members_data['membership_data']['payment_method'];
236+
// Update user source and add membership_role; a free plan always grants immediately.
237+
$members_data['defer_role'] = 'free' !== ( $members_data['membership_data']['type'] ?? 'unknown' )
238+
&& 'free' !== ( $members_data['membership_data']['payment_method'] ?? '' );
238239
$this->members_service->update_user_meta( $members_data, $member->ID );
239240

240241
$subscription_service = new SubscriptionService();

modules/membership/includes/Admin/Services/SubscriptionService.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,13 @@ public function get_membership_plan_details( $data ) {
329329

330330
if ( ! empty( $data['context'] ) && 'thank_you_page' === $data['context'] && ! empty( $data['transaction_id'] ) ) {
331331
$order_by_txn = $this->orders_repository->get_order_by_transaction_id( $data['transaction_id'] );
332-
if ( ! empty( $order_by_txn ) && ! empty( $order_by_txn['ID'] ) ) {
332+
333+
// Only trust this order if it belongs to the member the page is rendered for,
334+
// otherwise a submitted transaction_id could surface another member's order.
335+
if ( ! empty( $order_by_txn ) && ! empty( $order_by_txn['ID'] )
336+
&& isset( $order_by_txn['user_id'], $data['member_id'] )
337+
&& (int) $order_by_txn['user_id'] === (int) $data['member_id']
338+
) {
333339
$member_order = $order_by_txn;
334340
}
335341
}

modules/membership/includes/Templates/thank-you-page.php

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
$bank_data = ( isset( $_GET['info'] ) && ! empty( $_GET['info'] ) ) ? wp_kses_post( $_GET['info'] ) : '';
44
$show_bank_data = ( 'Free' === $bank_data || empty( $bank_data ) ) ? false : true;
5-
$transaction_id = ( isset( $_GET['transaction_id'] ) && ! empty( $_GET['transaction_id'] ) ) ? wp_kses_post( $_GET['transaction_id'] ) : '';
5+
$transaction_id = ( isset( $_GET['transaction_id'] ) && ! empty( $_GET['transaction_id'] ) ) ? sanitize_text_field( wp_unslash( $_GET['transaction_id'] ) ) : '';
66
$username = ( isset( $_GET['username'] ) && ! empty( $_GET['username'] ) ) ? wp_kses_post( $_GET['username'] ) : '';
77
$main_content = ! empty( $attributes['header'] ) ? wp_kses_post( $attributes['header'] ) : sprintf(
88
__( 'Thank You! Your registration was completed successfully.', 'user-registration' ),
@@ -48,20 +48,23 @@
4848
<div class="ur-message">
4949
<p>
5050
<?php
51-
$username = isset( $_GET['username'] ) ? sanitize_text_field( wp_unslash( $_GET['username'] ) ) : '';
52-
5351
$values = array();
5452

55-
if ( ! empty( $username ) ) {
56-
$user = get_user_by( 'login', sanitize_text_field( $username ) );
57-
$values['member_id'] = $user->ID;
58-
$values['email'] = $user->user_email;
59-
$values['context'] = 'thank_you_page';
60-
if ( ! empty( $transaction_id ) ) {
61-
$values['transaction_id'] = $transaction_id;
62-
}
53+
// Smart tags resolve only for the logged-in visitor's own account; a requested
54+
// username is never trusted, so one member cannot read another's details here.
55+
if ( is_user_logged_in() ) {
56+
$user = wp_get_current_user();
6357

64-
$main_content = apply_filters( 'user_registration_process_smart_tags', $main_content, $values );
58+
if ( $user && $user->exists() ) {
59+
$values['member_id'] = $user->ID;
60+
$values['email'] = $user->user_email;
61+
$values['context'] = 'thank_you_page';
62+
if ( ! empty( $transaction_id ) ) {
63+
$values['transaction_id'] = $transaction_id;
64+
}
65+
66+
$main_content = apply_filters( 'user_registration_process_smart_tags', $main_content, $values );
67+
}
6568
}
6669
echo wp_kses_post( $main_content );
6770
?>

0 commit comments

Comments
 (0)