Skip to content

Commit 13f29a8

Browse files
aristatharistath
authored
Merge pull request #662 from ProgressPlanner/ari/audit-2025-10
Audit & fixes
2 parents cc73b8c + 5caba6f commit 13f29a8

6 files changed

Lines changed: 1220 additions & 25 deletions

File tree

‎assets/js/web-components/prpl-task-sending-email.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,16 @@ customElements.define(
9696
'#prpl-sending-email-sent-message'
9797
).textContent = resultMessageText;
9898

99-
// Make AJAX GET request.
100-
fetch(
101-
prplEmailSending.ajax_url +
102-
'?action=prpl_test_email_sending&_wpnonce=' +
103-
prplEmailSending.nonce +
104-
'&email_address=' +
105-
emailAddress.value
106-
)
99+
// Make AJAX POST request.
100+
const formData = new FormData();
101+
formData.append( 'action', 'prpl_test_email_sending' );
102+
formData.append( 'email_address', emailAddress.value );
103+
formData.append( '_wpnonce', prplEmailSending.nonce );
104+
105+
fetch( prplEmailSending.ajax_url, {
106+
method: 'POST',
107+
body: formData,
108+
} )
107109
.then( ( response ) => response.json() )
108110
// eslint-disable-next-line no-unused-vars
109111
.then( ( response ) => {

‎classes/admin/class-page-settings.php‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,11 @@ public function store_settings_form_options() {
133133
\wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to update settings.', 'progress-planner' ) ] );
134134
}
135135

136-
// Check the nonce.
137-
\check_admin_referer( 'progress_planner' );
136+
// Use check_ajax_referer instead of check_admin_referer for AJAX handlers.
137+
// check_admin_referer is designed for form submissions, not AJAX requests.
138+
if ( ! \check_ajax_referer( 'progress_planner', 'nonce', false ) ) {
139+
\wp_send_json_error( [ 'message' => \esc_html__( 'Invalid nonce.', 'progress-planner' ) ] );
140+
}
138141

139142
if ( isset( $_POST['pages'] ) ) {
140143
foreach ( \wp_unslash( $_POST['pages'] ) as $type => $page_args ) { // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
@@ -189,11 +192,9 @@ public function store_settings_form_options() {
189192
* @return void
190193
*/
191194
public function save_settings() {
192-
// Check the nonce.
193-
\check_admin_referer( 'progress_planner' );
194-
195-
$redirect_on_login = isset( $_POST['prpl-redirect-on-login'] )
196-
? \sanitize_text_field( \wp_unslash( $_POST['prpl-redirect-on-login'] ) )
195+
// Nonce is already checked in store_settings_form_options() which calls this method.
196+
$redirect_on_login = isset( $_POST['prpl-redirect-on-login'] ) // phpcs:ignore WordPress.Security.NonceVerification.Missing
197+
? \sanitize_text_field( \wp_unslash( $_POST['prpl-redirect-on-login'] ) ) // phpcs:ignore WordPress.Security.NonceVerification.Missing
197198
: false;
198199

199200
\update_user_meta( \get_current_user_id(), 'prpl_redirect_on_login', (bool) $redirect_on_login );
@@ -205,11 +206,9 @@ public function save_settings() {
205206
* @return void
206207
*/
207208
public function save_post_types() {
208-
// Check the nonce.
209-
\check_admin_referer( 'progress_planner' );
210-
211-
$include_post_types = isset( $_POST['prpl-post-types-include'] )
212-
? \array_map( 'sanitize_text_field', \wp_unslash( $_POST['prpl-post-types-include'] ) ) // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
209+
// Nonce is already checked in store_settings_form_options() which calls this method.
210+
$include_post_types = isset( $_POST['prpl-post-types-include'] ) // phpcs:ignore WordPress.Security.NonceVerification.Missing
211+
? \array_map( 'sanitize_text_field', \wp_unslash( $_POST['prpl-post-types-include'] ) ) // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized, WordPress.Security.NonceVerification.Missing
213212
// If no post types are selected, use the default post types (post and page can be deregistered).
214213
: \array_intersect( [ 'post', 'page' ], \progress_planner()->get_settings()->get_public_post_types() );
215214

‎classes/class-suggested-tasks.php‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ public function was_task_completed( $task_id ): bool {
185185
* Primarly this is used for deeplinking, ie user is testing if the emails are working
186186
* He gets an email with a link which automatically completes the task.
187187
*
188+
* Verify token to prevent CSRF attacks.
189+
* Tokens are one-time use and expire after 24 hours.
190+
*
188191
* @return void
189192
*/
190193
public function maybe_complete_task() {
@@ -197,6 +200,19 @@ public function maybe_complete_task() {
197200
return;
198201
}
199202

203+
// Verify token to prevent CSRF attacks.
204+
if ( ! isset( $_GET['token'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
205+
return;
206+
}
207+
208+
$provided_token = \sanitize_text_field( \wp_unslash( $_GET['token'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended
209+
$user_id = \get_current_user_id();
210+
211+
// Verify the token.
212+
if ( ! $this->verify_task_completion_token( $task_id, $user_id, $provided_token ) ) {
213+
return;
214+
}
215+
200216
if ( ! $this->was_task_completed( $task_id ) ) {
201217
$task = \progress_planner()->get_suggested_tasks_db()->get_post( $task_id );
202218

@@ -205,10 +221,71 @@ public function maybe_complete_task() {
205221

206222
// Insert an activity.
207223
$this->insert_activity( $task_id );
224+
225+
// Delete the token after successful use (one-time use).
226+
$this->delete_task_completion_token( $task_id, $user_id );
208227
}
209228
}
210229
}
211230

231+
/**
232+
* Generate a secure token for task completion via email link.
233+
*
234+
* This token prevents CSRF attacks by ensuring only legitimate email
235+
* links can mark tasks as complete.
236+
*
237+
* @param string $task_id The task ID.
238+
* @param int $user_id The user ID.
239+
*
240+
* @return string The generated token.
241+
*/
242+
public function generate_task_completion_token( $task_id, $user_id ) {
243+
// Generate a cryptographically secure random token.
244+
$random = \wp_generate_password( 32, false );
245+
$token = \wp_hash( $task_id . $user_id . $random . \wp_salt( 'auth' ) );
246+
247+
// Store the token in a transient (expires after 24 hours).
248+
\set_transient(
249+
'prpl_complete_' . $task_id . '_' . $user_id,
250+
$token,
251+
DAY_IN_SECONDS
252+
);
253+
254+
return $token;
255+
}
256+
257+
/**
258+
* Verify a task completion token.
259+
*
260+
* @param string $task_id The task ID.
261+
* @param int $user_id The user ID.
262+
* @param string $provided_token The token to verify.
263+
*
264+
* @return bool True if token is valid, false otherwise.
265+
*/
266+
protected function verify_task_completion_token( $task_id, $user_id, $provided_token ) {
267+
$stored_token = \get_transient( 'prpl_complete_' . $task_id . '_' . $user_id );
268+
269+
if ( ! $stored_token ) {
270+
return false; // Token expired or doesn't exist.
271+
}
272+
273+
// Use hash_equals to prevent timing attacks.
274+
return \hash_equals( $stored_token, $provided_token );
275+
}
276+
277+
/**
278+
* Delete a task completion token after use.
279+
*
280+
* @param string $task_id The task ID.
281+
* @param int $user_id The user ID.
282+
*
283+
* @return bool True if deleted, false otherwise.
284+
*/
285+
protected function delete_task_completion_token( $task_id, $user_id ) {
286+
return \delete_transient( 'prpl_complete_' . $task_id . '_' . $user_id );
287+
}
288+
212289
/**
213290
* Handle the suggested task action.
214291
*

‎classes/suggested-tasks/providers/class-email-sending.php‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,15 @@ public function init() {
109109
\add_action( 'init', [ $this, 'check_if_wp_mail_has_override' ], PHP_INT_MAX );
110110

111111
$this->email_subject = \esc_html__( 'Your Progress Planner test message!', 'progress-planner' );
112+
113+
// Generate a secure token for the completion link to prevent CSRF.
114+
$user_id = \get_current_user_id();
115+
$token = \progress_planner()->get_suggested_tasks()->generate_task_completion_token( $this->get_task_id(), $user_id );
116+
112117
$this->email_content = \sprintf(
113118
// translators: %1$s the admin URL.
114119
\__( 'You just used Progress Planner to verify if sending email works on your website. <br><br> The good news; it does! <a href="%1$s" target="_self">Click here to mark %2$s\'s Recommendation as completed</a>.', 'progress-planner' ),
115-
\admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() ),
120+
\admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() . '&token=' . $token ),
116121
\esc_html( \progress_planner()->get_ui__branding()->get_ravi_name() )
117122
);
118123
}
@@ -248,6 +253,8 @@ protected function is_there_sending_email_override() {
248253
/**
249254
* Test email sending.
250255
*
256+
* Use check_ajax_referer and get email from $_POST.
257+
*
251258
* @return void
252259
*/
253260
public function ajax_test_email_sending() {
@@ -256,18 +263,32 @@ public function ajax_test_email_sending() {
256263
\wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to test email sending.', 'progress-planner' ) ] );
257264
}
258265

259-
// Check the nonce.
260-
\check_admin_referer( 'progress_planner' );
266+
// Use check_ajax_referer for AJAX handlers.
267+
if ( ! \check_ajax_referer( 'progress_planner', 'nonce', false ) ) {
268+
\wp_send_json_error( [ 'message' => \esc_html__( 'Invalid nonce.', 'progress-planner' ) ] );
269+
}
261270

262-
$email_address = isset( $_GET['email_address'] ) ? \sanitize_email( \wp_unslash( $_GET['email_address'] ) ) : '';
271+
// Get email from POST data (AJAX request).
272+
$email_address = isset( $_POST['email_address'] ) ? \sanitize_email( \wp_unslash( $_POST['email_address'] ) ) : '';
263273

264274
if ( ! $email_address ) {
265275
\wp_send_json_error( \esc_html__( 'Invalid email address.', 'progress-planner' ) );
266276
}
267277

278+
// Regenerate email content with fresh token for current user.
279+
$user_id = \get_current_user_id();
280+
$token = \progress_planner()->get_suggested_tasks()->generate_task_completion_token( $this->get_task_id(), $user_id );
281+
282+
$email_content = \sprintf(
283+
// translators: %1$s the admin URL.
284+
\__( 'You just used Progress Planner to verify if sending email works on your website. <br><br> The good news; it does! <a href="%1$s" target="_self">Click here to mark %2$s\'s Recommendation as completed</a>.', 'progress-planner' ),
285+
\admin_url( 'admin.php?page=progress-planner&prpl_complete_task=' . $this->get_task_id() . '&token=' . $token ),
286+
\esc_html( \progress_planner()->get_ui__branding()->get_ravi_name() )
287+
);
288+
268289
$headers = [ 'Content-Type: text/html; charset=UTF-8' ];
269290

270-
$result = \wp_mail( $email_address, $this->email_subject, $this->email_content, $headers );
291+
$result = \wp_mail( $email_address, $this->email_subject, $email_content, $headers );
271292

272293
if ( $result ) {
273294
\wp_send_json_success( \esc_html__( 'Email sent successfully.', 'progress-planner' ) );

‎classes/suggested-tasks/providers/class-tasks-interactive.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,18 @@ public function handle_interactive_task_submit() {
8888
$value = \sanitize_text_field( \wp_unslash( $_POST['value'] ) );
8989
$setting_path = \json_decode( \sanitize_text_field( \wp_unslash( $_POST['setting_path'] ) ), true );
9090

91+
// Whitelist allowed options to prevent arbitrary options update.
92+
// This prevents privilege escalation by restricting which options can be updated.
93+
$allowed_options = $this->get_allowed_interactive_options();
94+
95+
if ( ! \in_array( $setting, $allowed_options, true ) ) {
96+
\wp_send_json_error(
97+
[
98+
'message' => \esc_html__( 'Invalid setting. This option cannot be updated through interactive tasks.', 'progress-planner' ),
99+
]
100+
);
101+
}
102+
91103
if ( ! empty( $setting_path ) ) {
92104
$setting_value = \get_option( $setting );
93105
\_wp_array_set( $setting_value, $setting_path, $value );
@@ -105,6 +117,47 @@ public function handle_interactive_task_submit() {
105117
\wp_send_json_success( [ 'message' => \esc_html__( 'Setting updated.', 'progress-planner' ) ] );
106118
}
107119

120+
/**
121+
* Get the list of allowed options that can be updated via interactive tasks.
122+
*
123+
* This whitelist prevents privilege escalation by ensuring only specific
124+
* WordPress options can be modified through the interactive task interface.
125+
*
126+
* @return array List of allowed option names.
127+
*/
128+
protected function get_allowed_interactive_options() {
129+
$allowed_options = [
130+
// Core WordPress settings that are safe to update via interactive tasks.
131+
'blogdescription', // Site tagline.
132+
'default_comment_status', // Comment settings.
133+
'default_ping_status', // Pingback settings.
134+
'timezone_string', // Site timezone.
135+
'WPLANG', // Site language/locale (deprecated since WP 4.0, but still used by class-select-locale.php).
136+
'date_format', // Date format.
137+
'time_format', // Time format.
138+
'default_pingback_flag', // Pingback flag.
139+
'comment_registration', // Comment registration.
140+
'close_comments_for_old_posts', // Close comments for old posts.
141+
'thread_comments', // Threaded comments.
142+
'comments_per_page', // Comments per page.
143+
'comment_order', // Comment order.
144+
'page_comments', // Paginate comments.
145+
];
146+
147+
/**
148+
* Filter the list of allowed options for interactive tasks.
149+
*
150+
* WARNING: Be very careful when extending this list. Adding sensitive
151+
* options like 'admin_email', 'users_can_register', or plugin-specific
152+
* options that control access or permissions could create security vulnerabilities.
153+
*
154+
* @param array $allowed_options List of allowed option names.
155+
*
156+
* @return array Modified list of allowed option names.
157+
*/
158+
return \apply_filters( 'progress_planner_interactive_task_allowed_options', $allowed_options );
159+
}
160+
108161
/**
109162
* Add the popover.
110163
*

0 commit comments

Comments
 (0)