Skip to content

Commit 75c940d

Browse files
MILAN88888MILAN88888
authored
#1408 Fix - Prevent membership privilege escalation and open redirect after login (#1422)
* #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
1 parent e52cf15 commit 75c940d

8 files changed

Lines changed: 417 additions & 81 deletions

File tree

‎includes/Functions/CoreFunctions.php‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,129 @@ function ur_membership_verify_nonce( $nonce ) {
9595
}
9696
}
9797

98+
if ( ! function_exists( 'ur_membership_get_privileged_capabilities' ) ) {
99+
/**
100+
* Capabilities that make a role unsafe to grant automatically through a membership.
101+
*
102+
* The stock editor role holds unfiltered_html and WooCommerce's shop_manager holds list_users,
103+
* so neither is listed here: both are roles a site may legitimately attach to a plan.
104+
*
105+
* @since 5.2.8
106+
*
107+
* @return array List of capability names.
108+
*/
109+
function ur_membership_get_privileged_capabilities() {
110+
/**
111+
* Filters the capabilities that bar a role from being granted through a membership.
112+
*
113+
* @since 5.2.8
114+
*
115+
* @param array $capabilities List of capability names.
116+
*/
117+
return apply_filters(
118+
'user_registration_membership_privileged_capabilities',
119+
array(
120+
'manage_options',
121+
'promote_users',
122+
'edit_users',
123+
'create_users',
124+
'delete_users',
125+
'remove_users',
126+
'install_plugins',
127+
'activate_plugins',
128+
'update_plugins',
129+
'edit_plugins',
130+
'install_themes',
131+
'switch_themes',
132+
'edit_themes',
133+
'edit_files',
134+
'edit_dashboard',
135+
)
136+
);
137+
}
138+
}
139+
140+
if ( ! function_exists( 'ur_membership_is_privileged_role' ) ) {
141+
/**
142+
* Whether a role holds a capability that makes it unsafe to grant through a membership.
143+
*
144+
* @since 5.2.8
145+
*
146+
* @param string $role Role slug.
147+
* @return bool True when the role is too privileged to grant automatically.
148+
*/
149+
function ur_membership_is_privileged_role( $role ) {
150+
$role_object = wp_roles()->get_role( sanitize_key( $role ) );
151+
152+
if ( ! $role_object ) {
153+
return false;
154+
}
155+
156+
foreach ( ur_membership_get_privileged_capabilities() as $capability ) {
157+
if ( ! empty( $role_object->capabilities[ $capability ] ) ) {
158+
return true;
159+
}
160+
}
161+
162+
return false;
163+
}
164+
}
165+
166+
if ( ! function_exists( 'ur_membership_get_safe_role' ) ) {
167+
/**
168+
* Constrain the role a membership grants, as a backstop at the point of assignment.
169+
*
170+
* A plan's role is administrator-authored data, so a privileged value is honoured when the
171+
* plan was authored by someone entitled to assign roles, and refused otherwise. That keeps a
172+
* plan injected by a lower role from granting itself anything, without overriding a choice an
173+
* administrator deliberately made. Every refusal is logged, because a silent downgrade reads
174+
* as the membership simply not working.
175+
*
176+
* @since 5.2.8
177+
*
178+
* @param string $role Role slug taken from the membership data.
179+
* @param int $membership_id Membership post ID the role came from.
180+
* @param string $fallback Role used when the requested one is missing or refused.
181+
* @return string Role slug safe to grant.
182+
*/
183+
function ur_membership_get_safe_role( $role, $membership_id = 0, $fallback = 'subscriber' ) {
184+
$role = sanitize_key( $role );
185+
$safe = $role;
186+
187+
if ( empty( $role ) || ! wp_roles()->is_role( $role ) ) {
188+
$safe = $fallback;
189+
} elseif ( ur_membership_is_privileged_role( $role ) ) {
190+
$author_id = $membership_id ? (int) get_post_field( 'post_author', absint( $membership_id ) ) : 0;
191+
192+
if ( ! $author_id || ! user_can( $author_id, 'promote_users' ) ) {
193+
$safe = $fallback;
194+
195+
ur_get_logger()->warning(
196+
sprintf(
197+
/* translators: 1: requested role slug, 2: membership ID, 3: role granted instead. */
198+
'Refused to grant privileged role "%1$s" from membership %2$d because its author cannot assign roles; granted "%3$s" instead.',
199+
$role,
200+
absint( $membership_id ),
201+
$fallback
202+
),
203+
array( 'source' => 'user-registration-membership' )
204+
);
205+
}
206+
}
207+
208+
/**
209+
* Filters the role a membership grants, after the privilege backstop has run.
210+
*
211+
* @since 5.2.8
212+
*
213+
* @param string $safe Role slug that will be granted.
214+
* @param string $role Role slug requested by the membership data.
215+
* @param int $membership_id Membership post ID the role came from.
216+
*/
217+
return apply_filters( 'user_registration_membership_safe_role', $safe, $role, $membership_id );
218+
}
219+
}
220+
98221
if ( ! function_exists( 'ur_membership_get_currencies' ) ) {
99222
/**
100223
* ur_membership_get_currencies

‎includes/frontend/class-ur-frontend.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public function __construct() {
4545
add_filter( 'user_registration_my_account_shortcode', array( $this, 'user_registration_my_account_layout' ) );
4646
add_filter( 'user_registration_before_save_profile_details', array( $this, 'user_registration_before_save_profile_details' ), 10, 3 );
4747
add_filter( 'user_registration_login_redirect', array( $this, 'login_redirect' ), 10, 2 );
48+
add_filter( 'allowed_redirect_hosts', array( $this, 'allow_custom_redirect_host' ) );
4849
add_filter( 'user_registration_redirect_after_logout', array( $this, 'logout_redirect' ), 10, 1 );
4950
add_action( 'init', array( $this, 'ur_register_payment_tab_if_eligible' ) );
5051
}
@@ -233,6 +234,41 @@ public function login_redirect( $redirect, $user ) {
233234
}
234235
return apply_filters( 'user_registration_login_redirect_url', $redirect, $user, $redirect_option );
235236
}
237+
/**
238+
* Allow the admin-configured external login redirect host through wp_validate_redirect().
239+
*
240+
* The after-login redirect is validated against the allowed hosts, so the External URL set
241+
* in Login Options has to be listed or that setting would fall back to the home page.
242+
*
243+
* @since 5.2.8
244+
*
245+
* @param string[] $hosts Allowed redirect host names.
246+
* @return string[] Allowed redirect host names.
247+
*/
248+
public function allow_custom_redirect_host( $hosts ) {
249+
if ( ! ur_string_to_bool( get_option( 'user_registration_login_options_enable_custom_redirect', false ) ) ) {
250+
return $hosts;
251+
}
252+
253+
if ( 'external-url' !== get_option( 'user_registration_login_options_redirect_after_login', 'no-redirection' ) ) {
254+
return $hosts;
255+
}
256+
257+
$external_url = get_option( 'user_registration_login_options_after_login_redirect_external_url', '' );
258+
259+
if ( empty( $external_url ) || ! ur_is_valid_url( $external_url ) ) {
260+
return $hosts;
261+
}
262+
263+
$host = wp_parse_url( $external_url, PHP_URL_HOST );
264+
265+
if ( ! empty( $host ) && ! in_array( $host, $hosts, true ) ) {
266+
$hosts[] = $host;
267+
}
268+
269+
return $hosts;
270+
}
271+
236272
public function logout_redirect( $redirect ) {
237273
if ( ! ur_string_to_bool( get_option( 'user_registration_login_options_enable_custom_redirect', false ) ) ) {
238274
return $redirect;

‎includes/functions-ur-core.php‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5401,17 +5401,15 @@ function ur_process_login( $nonce_value ) {
54015401
*/
54025402
$redirect = apply_filters( 'user_registration_login_redirect', $redirect, $user );
54035403

5404+
// Validate above the branch: the AJAX response is navigated to client side, so a fix on the header redirect alone would not cover it.
5405+
$redirect = wp_validate_redirect( $redirect, get_home_url() );
5406+
54045407
if ( ur_is_ajax_login_enabled() && empty( $_POST['resubmitted'] ) ) { // phpcs:ignore
54055408
wp_send_json_success( array( 'message' => $redirect ) );
5406-
wp_send_json( $user );
54075409
} else {
5408-
wp_redirect( wp_validate_redirect( $redirect, $redirect ) ); // phpcs:ignore
5410+
wp_safe_redirect( $redirect );
54095411
exit;
54105412
}
5411-
5412-
if ( ur_is_ajax_login_enabled() && empty( $_POST['resubmitted'] ) ) { // phpcs:ignore
5413-
wp_send_json( $user );
5414-
}
54155413
}
54165414
} catch ( Exception $e ) {
54175415
$status_code = $e->getCode();
@@ -5459,9 +5457,12 @@ function ( $err_msg ) use ( $message ) {
54595457
*/
54605458
do_action( 'user_registration_login_failed' );
54615459

5462-
$redirect_url = wp_get_raw_referer() ? wp_get_raw_referer() : ur_get_my_account_url();
5460+
$referer = wp_get_raw_referer();
5461+
5462+
// wp_validate_redirect() returns an empty string for empty input, not the fallback, so guard it explicitly.
5463+
$redirect_url = $referer ? wp_validate_redirect( $referer, ur_get_my_account_url() ) : ur_get_my_account_url();
54635464
$redirect_url = add_query_arg( 'urm_error', $error_key, $redirect_url );
5464-
wp_redirect( $redirect_url );
5465+
wp_safe_redirect( $redirect_url );
54655466
exit;
54665467

54675468
}

‎modules/membership/includes/AJAX.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1818,6 +1818,16 @@ public static function upgrade_membership() {
18181818
);
18191819
}
18201820

1821+
// Only a plan the site currently offers may be selected.
1822+
// phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce verified by ur_membership_verify_nonce() at the top of this handler.
1823+
if ( ! ( new MembershipService() )->is_membership_purchasable( absint( $_POST['selected_membership_id'] ) ) ) {
1824+
wp_send_json_error(
1825+
array(
1826+
'message' => __( 'Invalid membership selected.', 'user-registration' ),
1827+
)
1828+
);
1829+
}
1830+
18211831
if ( isset( $_POST['form_data'] ) && ! empty( $_POST['form_data'] ) ) {
18221832
$single_field = array();
18231833
$form_data = json_decode( wp_unslash( $_POST['form_data'] ) );
@@ -2075,6 +2085,16 @@ public static function add_multiple_membership() {
20752085
);
20762086
}
20772087

2088+
// Only a plan the site currently offers may be selected.
2089+
// phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce verified by ur_membership_verify_nonce() at the top of this handler.
2090+
if ( ! ( new MembershipService() )->is_membership_purchasable( absint( $_POST['selected_membership_id'] ) ) ) {
2091+
wp_send_json_error(
2092+
array(
2093+
'message' => __( 'Invalid membership selected.', 'user-registration' ),
2094+
)
2095+
);
2096+
}
2097+
20782098
if ( isset( $_POST['form_data'] ) && ! empty( $_POST['form_data'] ) ) {
20792099
$single_field = array();
20802100
$form_data = json_decode( wp_unslash( $_POST['form_data'] ) );
@@ -2205,6 +2225,15 @@ function ( $user_memberships ) {
22052225
$membership_meta = json_decode( wp_unslash( $membership_data['meta_value'] ), true );
22062226
$membership_type = $membership_meta['type'] ?? 'unknown'; // free, paid, or subscription
22072227

2228+
// Reject a payment method the plan does not accept: 'free' on a paid plan would grant the plan's role at once, unpaid.
2229+
if ( ! ( new MembershipService() )->is_valid_payment_method_for_membership( $membership_meta, $data['payment_method'], $data ) ) {
2230+
wp_send_json_error(
2231+
array(
2232+
'message' => __( 'Invalid payment method for this membership.', 'user-registration' ),
2233+
)
2234+
);
2235+
}
2236+
22082237
$payment_gateway = $data['payment_method'] ?? 'unknown';
22092238
$member_id = get_current_user_id();
22102239
$member = get_userdata( $member_id );

0 commit comments

Comments
 (0)