wordpress-turnstile

commit 343da3d34a13638db879bc0ccf48035b54073271
parent a19b1ee0ac414306a71b0e236be7431cd101cc09
Author: Christian Grothoff <christian@grothoff.org>
Date:   Sat, 15 Nov 2025 15:53:27 +0100

address some sanitization requests

Diffstat:
M
admin/class-admin-settings.php
 | 
8
++++++++
M
admin/class-price-category-admin.php
 | 
43
+++++++++++++++++++++++++++++++------------
M
includes/class-field-manager.php
 | 
5
+++++

3 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/admin/class-admin-settings.php b/admin/class-admin-settings.php
@@ -265,6 +265,14 @@ class Taler_Turnstile_Admin_Settings {
         return sanitize_text_field($input);
     }
 
+    /**
+     * Sanitizer for the "boolean" grant access option as
+     * requested by WP reviewers. Not sure how to really
+     * validate that an input of type boolean is an input,
+     * but the Internets suggest to check that the boolean
+     * isset(), which makes some sense. So we check that the
+     * $input isset().
+     */
     public function sanitize_grant_access_option($input) {
       return isset($input);
     }
diff --git a/admin/class-price-category-admin.php b/admin/class-price-category-admin.php
@@ -183,10 +183,6 @@ class Taler_Turnstile_Price_Category_Admin {
         $label = isset($_POST['label']) ? sanitize_text_field($_POST['label']) : '';
         // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash
         $description = isset($_POST['description']) ? sanitize_textarea_field($_POST['description']) : '';
-        // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash
-        // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
-        // FIXME: reviewers say sanitization is needed here!??
-        $prices = isset($_POST['prices']) ? $_POST['prices'] : array();
 
         // Determine if this is an edit or new category
         $id = isset($_POST['id']) ? sanitize_key($_POST['id']) : '';
@@ -196,25 +192,29 @@ class Taler_Turnstile_Price_Category_Admin {
             $id = $this->generate_unique_id();
         }
 
+        // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash
+        // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
+        // Note: reviewers suggested sanitization is needed here;
+        // alas, the sanitization happens just below in the loops.
+        $prices = isset($_POST['prices']) ? $_POST['prices'] : array();
+
         // Validate and filter prices
         $filtered_prices = array();
-        $sub_expirations = array();
-
         if (is_array($prices)) {
             foreach ($prices as $subscription_id => $currencies) {
                 $sub_active = FALSE;
+                if (! check_valid_subscription_id ($subscription_id))
+                    continue; // ignore
                 if (is_array($currencies)) {
                     foreach ($currencies as $currency_code => $price) {
+                        if (! check_valid_currency_code ($currency_code))
+                            continue; // ignore
                         if ($price !== '' && is_numeric($price) && $price >= 0) {
                             $filtered_prices[$subscription_id][$currency_code] = floatval($price);
                             $sub_active = TRUE;
                         }
                     }
                 }
-                if ($sub_active) {
-                    $sub_expirations[$subscription_id]
-                      = $subscriptions[$subscription_id]['valid_before_s'];
-                }
             }
         }
 
@@ -222,8 +222,6 @@ class Taler_Turnstile_Price_Category_Admin {
             'label' => $label,
             'description' => $description,
             'prices' => $filtered_prices,
-            // FIXME: expirations / sub_expirations not used!!?
-            'expirations' => $sub_expirations
         );
 
         Taler_Price_Category::save($id, $category_data);
@@ -262,6 +260,27 @@ class Taler_Turnstile_Price_Category_Admin {
     }
 
     /**
+     * Check if the given string is a valid Taler currency code.
+     * Accepted are [a-z][A-Z], 1 to 11 characters long.
+     *
+     * @return boolean true if $cc is valid.
+     */
+    private function check_valid_currency_code ($cc) {
+         return preg_match('/^[a-zA-Z]{1,11}$/', $cc) === 1;
+    }
+
+    /**
+     * Check if the given string is a valid $id, consisting
+     * of only unreserved characters as per RFC 3986.
+     *
+     * @return boolean true if $id is valid.
+     */
+    private function check_valid_subscription_id ($id) {
+        // RFC 3986 unreserved characters are A–Z a–z 0–9 - . _ ~
+        return preg_match('/^[A-Za-z0-9\-._~]+$/', $id) === 1;
+    }
+
+    /**
      * Generate a unique ID for a new price category
      *
      * @return string Unique ID
diff --git a/includes/class-field-manager.php b/includes/class-field-manager.php
@@ -102,6 +102,11 @@ class Taler_Field_Manager {
         // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
         // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash
         // FIXME: reviewers say sanitization is needed here for wp_verify_nonce()
+        // But https://wordpress.stackexchange.com/questions/256513/should-nonce-be-sanitized
+        // says this is not needed and points to
+        // https://github.com/WordPress/wordpress-develop/blob/6.2/src/wp-includes/pluggable.php#L1271
+        // for an example where the wordpress core also does not do it.
+        // I truly do not see why the reviewers believe sanitization is useful here.
         if (!isset($_POST['taler_price_category_nonce']) ||
             !wp_verify_nonce($_POST['taler_price_category_nonce'], 'taler_price_category_meta')) {
             return;