Changeset 59322 for branches/6.7

Timestamp:

10/29/2024 04:33:51 PM (5 weeks ago)

Author:

SergeyBiryukov

Message:

Comments: Use a more precise check for disallowed keys on filtered comment data.

The previous approach of running wp_allow_comment() twice could have unintended consequences, e.g. the check_comment_flood action was also triggered twice, which might lead to false-positive identification of comment flood in case there is some custom callback hooked to it, which is not expecting identical data seeing twice.

This commit introduces a new function, wp_check_comment_data(), to specifically check for disallowed content before and after comment data is filtered.

Follow-up to [59267].

Reviewed by davidbaumwald.
Merges [59319] to the 6.7 branch.

Props david.binda, SergeyBiryukov.
Fixes #61827.

Location:

branches/6.7

Files:

• . (modified) (1 prop)
• src/wp-includes/comment.php (modified) (3 diffs)
• tests/phpunit/tests/comment/wpHandleCommentSubmission.php (modified) (2 diffs)

branches/6.7/src/wp-includes/comment.php

@@ -774 +774 @@
     }
 
-    if ( ! empty( $commentdata['user_id'] ) ) {
-        $user        = get_userdata( $commentdata['user_id'] );
-        $post_author = $wpdb->get_var(
-            $wpdb->prepare(
-                "SELECT post_author FROM $wpdb->posts WHERE ID = %d LIMIT 1",
-                $commentdata['comment_post_ID']
-            )
-        );
-    }
-
-    if ( isset( $user ) && ( $commentdata['user_id'] == $post_author || $user->has_cap( 'moderate_comments' ) ) ) {
-        // The author and the admins get respect.
-        $approved = 1;
-    } else {
-        // Everyone else's comments will be checked.
-        if ( check_comment(
-            $commentdata['comment_author'],
-            $commentdata['comment_author_email'],
-            $commentdata['comment_author_url'],
-            $commentdata['comment_content'],
-            $commentdata['comment_author_IP'],
-            $commentdata['comment_agent'],
-            $commentdata['comment_type']
-        ) ) {
-            $approved = 1;
-        } else {
-            $approved = 0;
-        }
-
-        if ( wp_check_comment_disallowed_list(
-            $commentdata['comment_author'],
-            $commentdata['comment_author_email'],
-            $commentdata['comment_author_url'],
-            $commentdata['comment_content'],
-            $commentdata['comment_author_IP'],
-            $commentdata['comment_agent']
-        ) ) {
-            $approved = EMPTY_TRASH_DAYS ? 'trash' : 'spam';
-        }
-    }
-
-    /**
-     * Filters a comment's approval status before it is set.
-     *
-     * @since 2.1.0
-     * @since 4.9.0 Returning a WP_Error value from the filter will short-circuit comment insertion
-     *              and allow skipping further processing.
-     *
-     * @param int|string|WP_Error $approved    The approval status. Accepts 1, 0, 'spam', 'trash',
-     *                                         or WP_Error.
-     * @param array               $commentdata Comment data.
-     */
-    return apply_filters( 'pre_comment_approved', $approved, $commentdata );
+    return wp_check_comment_data( $commentdata );
 }
 
@@ -1291 +1239 @@
 
     return true;
+}
+
+/**
+ * Checks whether comment data passes internal checks or has disallowed content.
+ *
+ * @since 6.7.0
+ *
+ * @global wpdb $wpdb WordPress database abstraction object.
+ *
+ * @param array $comment_data Array of arguments for inserting a comment.
+ * @return int|string|WP_Error The approval status on success (0|1|'spam'|'trash'),
+  *                            WP_Error otherwise.
+ */
+function wp_check_comment_data( $comment_data ) {
+    global $wpdb;
+
+    if ( ! empty( $comment_data['user_id'] ) ) {
+        $user        = get_userdata( $comment_data['user_id'] );
+        $post_author = $wpdb->get_var(
+            $wpdb->prepare(
+                "SELECT post_author FROM $wpdb->posts WHERE ID = %d LIMIT 1",
+                $comment_data['comment_post_ID']
+            )
+        );
+    }
+
+    if ( isset( $user ) && ( $comment_data['user_id'] == $post_author || $user->has_cap( 'moderate_comments' ) ) ) {
+        // The author and the admins get respect.
+        $approved = 1;
+    } else {
+        // Everyone else's comments will be checked.
+        if ( check_comment(
+            $comment_data['comment_author'],
+            $comment_data['comment_author_email'],
+            $comment_data['comment_author_url'],
+            $comment_data['comment_content'],
+            $comment_data['comment_author_IP'],
+            $comment_data['comment_agent'],
+            $comment_data['comment_type']
+        ) ) {
+            $approved = 1;
+        } else {
+            $approved = 0;
+        }
+
+        if ( wp_check_comment_disallowed_list(
+            $comment_data['comment_author'],
+            $comment_data['comment_author_email'],
+            $comment_data['comment_author_url'],
+            $comment_data['comment_content'],
+            $comment_data['comment_author_IP'],
+            $comment_data['comment_agent']
+        ) ) {
+            $approved = EMPTY_TRASH_DAYS ? 'trash' : 'spam';
+        }
+    }
+
+    /**
+     * Filters a comment's approval status before it is set.
+     *
+     * @since 2.1.0
+     * @since 4.9.0 Returning a WP_Error value from the filter will short-circuit comment insertion
+     *              and allow skipping further processing.
+     *
+     * @param int|string|WP_Error $approved    The approval status. Accepts 1, 0, 'spam', 'trash',
+     *                                         or WP_Error.
+     * @param array               $commentdata Comment data.
+     */
+    return apply_filters( 'pre_comment_approved', $approved, $comment_data );
 }
 
@@ -2280 +2297 @@
     $commentdata['comment_approved'] = wp_allow_comment( $commentdata, $wp_error );
 
+    if ( is_wp_error( $commentdata['comment_approved'] ) ) {
+        return $commentdata['comment_approved'];
+    }
+
     $commentdata = wp_filter_comment( $commentdata );
 
     if ( ! in_array( $commentdata['comment_approved'], array( 'trash', 'spam' ), true ) ) {
         // Validate the comment again after filters are applied to comment data.
-        $commentdata['comment_approved'] = wp_allow_comment( $commentdata, $wp_error );
+        $commentdata['comment_approved'] = wp_check_comment_data( $commentdata );
     }
 

branches/6.7/tests/phpunit/tests/comment/wpHandleCommentSubmission.php

@@ -990 +990 @@
         $comment = wp_handle_comment_submission( $data );
 
-        $this->assertNotWPError( $comment );
-        $this->assertInstanceOf( 'WP_Comment', $comment );
-        $this->assertSame( 'trash', $comment->comment_approved );
+        $this->assertInstanceOf( 'WP_Comment', $comment, 'The comment was not submitted.' );
+        $this->assertSame( 'trash', $comment->comment_approved, 'The wrong approved status was returned.' );
     }
 
@@ -1010 +1009 @@
         $comment = wp_handle_comment_submission( $data );
 
-        $this->assertNotWPError( $comment );
-        $this->assertInstanceOf( 'WP_Comment', $comment );
-        $this->assertSame( 'trash', $comment->comment_approved );
+        $this->assertInstanceOf( 'WP_Comment', $comment, 'The comment was not submitted.' );
+        $this->assertSame( 'trash', $comment->comment_approved, 'The wrong approved status was returned.' );
+    }
+
+    /**
+     * @ticket 61827
+     */
+    public function test_disallowed_keys_filtered_html_match_does_not_call_check_comment_flood_action_twice() {
+        $data = array(
+            'comment_post_ID' => self::$post->ID,
+            'comment'         => '<a href=http://example.com/>example</a>',
+            'author'          => 'Comment Author',
+            'email'           => 'comment@example.org',
+        );
+
+        update_option( 'disallowed_keys', "href=\\\"http\nfoo" );
+
+        $pre_comment_approved = new MockAction();
+        $check_comment_flood  = new MockAction();
+        add_filter( 'pre_comment_approved', array( $pre_comment_approved, 'filter' ), 10, 2 );
+        add_action( 'check_comment_flood', array( $check_comment_flood, 'action' ), 10, 4 );
+
+        $comment = wp_handle_comment_submission( $data );
+
+        $this->assertInstanceOf( 'WP_Comment', $comment, 'The comment was not submitted.' );
+        $this->assertSame( 'trash', $comment->comment_approved, 'The wrong approved status was returned.' );
+
+        $this->assertSame( 2, $pre_comment_approved->get_call_count(), 'The `pre_comment_approved` filter was not called twice.' );
+        $this->assertSame( 1, $check_comment_flood->get_call_count(), 'The `check_comment_flood` action was not called exactly once.' );
     }
 }