Skip to content

Commit a1b98d4

Browse files
aristatharistath
authored
Merge pull request #680 from ProgressPlanner/ari/improve-security
Additional checks and tweaks to strengthen security
2 parents 80c7c30 + 788db8a commit a1b98d4

5 files changed

Lines changed: 103 additions & 46 deletions

File tree

‎classes/activities/class-query.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,9 +496,11 @@ private function upgrade_20241011() {
496496

497497
$table_name = $wpdb->prefix . static::TABLE_NAME; // @phpstan-ignore-line property.nonObject
498498

499-
foreach ( $wpdb->get_results( "DESCRIBE $table_name" ) as $column ) {
499+
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching
500+
foreach ( $wpdb->get_results( $wpdb->prepare( 'DESCRIBE %i', $table_name ) ) as $column ) { // @phpstan-ignore-line
500501
if ( 'data_id' === $column->Field && \str_contains( \strtolower( $column->Type ), 'int' ) ) { // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase, @phpstan-ignore-line property.nonObject
501-
$wpdb->query( "ALTER TABLE $table_name CHANGE COLUMN data_id data_id VARCHAR(255)" );
502+
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.DirectDatabaseQuery.SchemaChange
503+
$wpdb->query( $wpdb->prepare( 'ALTER TABLE %i CHANGE COLUMN data_id data_id VARCHAR(255)', $table_name ) ); // @phpstan-ignore-line
502504
}
503505
}
504506
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,11 @@ public function store_settings_form_options() {
140140
}
141141

142142
if ( isset( $_POST['pages'] ) ) {
143-
foreach ( \wp_unslash( $_POST['pages'] ) as $type => $page_args ) { // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
143+
// Sanitize the pages array at point of reception.
144+
$pages = \map_deep( \wp_unslash( $_POST['pages'] ), 'sanitize_text_field' ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
144145

145-
$need_page = \sanitize_text_field( \wp_unslash( $page_args['have_page'] ) );
146+
foreach ( $pages as $type => $page_args ) {
147+
$need_page = isset( $page_args['have_page'] ) ? $page_args['have_page'] : '';
146148

147149
\progress_planner()->get_page_types()->set_no_page_needed(
148150
$type,

‎classes/rest/class-base.php‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
<?php
2+
/**
3+
* Base class for REST API endpoints.
4+
*
5+
* @package Progress_Planner
6+
*/
7+
8+
namespace Progress_Planner\Rest;
9+
10+
/**
11+
* Base class for REST API endpoints.
12+
*/
13+
abstract class Base {
14+
15+
/**
16+
* Get client IP address.
17+
*
18+
* @return string
19+
*/
20+
protected function get_client_ip() {
21+
$ip_keys = [
22+
'HTTP_CF_CONNECTING_IP', // Cloudflare.
23+
'HTTP_X_REAL_IP', // Nginx proxy.
24+
'HTTP_X_FORWARDED_FOR', // Standard proxy header.
25+
'REMOTE_ADDR', // Direct connection.
26+
];
27+
28+
foreach ( $ip_keys as $key ) {
29+
if ( isset( $_SERVER[ $key ] ) && ! empty( $_SERVER[ $key ] ) ) {
30+
$ip = \sanitize_text_field( \wp_unslash( $_SERVER[ $key ] ) );
31+
// Handle X-Forwarded-For which may contain multiple IPs.
32+
if ( \strpos( $ip, ',' ) !== false ) {
33+
$ip = \trim( \explode( ',', $ip )[0] );
34+
}
35+
// Validate IP address.
36+
if ( \filter_var( $ip, FILTER_VALIDATE_IP ) ) {
37+
return $ip;
38+
}
39+
}
40+
}
41+
42+
return '0.0.0.0';
43+
}
44+
45+
/**
46+
* Validate the token.
47+
*
48+
* @param string $token The token.
49+
*
50+
* @return bool
51+
*/
52+
public function validate_token( $token ) {
53+
// Rate limiting: Check for too many failed attempts.
54+
$ip_address = $this->get_client_ip();
55+
$rate_limit_key = 'prpl_api_rate_limit_' . \md5( $ip_address );
56+
$failed_attempts = (int) \get_transient( $rate_limit_key );
57+
58+
// Block if more than 10 failed attempts in the last hour.
59+
if ( $failed_attempts >= 10 ) {
60+
return false;
61+
}
62+
63+
$token = \str_replace( 'token/', '', $token );
64+
65+
// Check test token first (use hash_equals to prevent timing attacks).
66+
$test_token = \get_option( 'progress_planner_test_token', '' );
67+
if ( ! empty( $test_token ) && \hash_equals( $test_token, $token ) ) {
68+
// Clear failed attempts on successful authentication.
69+
\delete_transient( $rate_limit_key );
70+
return true;
71+
}
72+
73+
$license_key = \get_option( 'progress_planner_license_key', false );
74+
if ( ! $license_key || 'no-license' === $license_key ) {
75+
// Increment failed attempts counter.
76+
\set_transient( $rate_limit_key, $failed_attempts + 1, HOUR_IN_SECONDS );
77+
return false;
78+
}
79+
80+
// Use hash_equals() to prevent timing attacks.
81+
$is_valid = \hash_equals( $license_key, $token );
82+
83+
if ( ! $is_valid ) {
84+
// Increment failed attempts counter.
85+
\set_transient( $rate_limit_key, $failed_attempts + 1, HOUR_IN_SECONDS );
86+
} else {
87+
// Clear failed attempts on successful authentication.
88+
\delete_transient( $rate_limit_key );
89+
}
90+
91+
return $is_valid;
92+
}
93+
}

‎classes/rest/class-stats.php‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@
1212

1313
namespace Progress_Planner\Rest;
1414

15-
use Progress_Planner\Base;
1615
use Progress_Planner\Admin\Widgets\Activity_Scores;
1716

1817
/**
1918
* Rest_API_Stats class.
2019
*/
21-
class Stats {
20+
class Stats extends Base {
2221
/**
2322
* Constructor.
2423
*/
@@ -63,21 +62,4 @@ public function get_stats() {
6362

6463
return new \WP_REST_Response( $system_status->get_system_status() );
6564
}
66-
67-
/**
68-
* Validate the token.
69-
*
70-
* @param string $token The token.
71-
*
72-
* @return bool
73-
*/
74-
public function validate_token( $token ) {
75-
$token = \str_replace( 'token/', '', $token );
76-
$license_key = \get_option( 'progress_planner_license_key', false );
77-
if ( ! $license_key || 'no-license' === $license_key ) {
78-
return false;
79-
}
80-
81-
return $token === $license_key;
82-
}
8365
}

‎classes/rest/class-tasks.php‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
/**
1414
* Rest_API_Tasks class.
1515
*/
16-
class Tasks {
16+
class Tasks extends Base {
1717
/**
1818
* Constructor.
1919
*/
@@ -46,28 +46,6 @@ public function register_rest_endpoint() {
4646
);
4747
}
4848

49-
/**
50-
* Permission callback.
51-
*
52-
* @param string $token The token.
53-
*
54-
* @return bool
55-
*/
56-
public function validate_token( $token ) {
57-
$token = \str_replace( 'token/', '', $token );
58-
59-
if ( $token === \get_option( 'progress_planner_test_token', '' ) ) {
60-
return true;
61-
}
62-
63-
$license_key = \get_option( 'progress_planner_license_key', false );
64-
if ( ! $license_key || 'no-license' === $license_key ) {
65-
return false;
66-
}
67-
68-
return $token === $license_key;
69-
}
70-
7149
/**
7250
* Get task recommendations.
7351
*

0 commit comments

Comments
 (0)