Skip to content

Commit

Permalink
Comments: Use a more precise check for disallowed keys on filtered co…
Browse files Browse the repository at this point in the history 
…mment 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].

Props david.binda, SergeyBiryukov.
See #61827.

git-svn-id: https://develop.svn.wordpress.org/trunk@59319 602fd350-edb4-49c9-b593-d223f7449a82
  • Loading branch information
@SergeyBiryukov
SergeyBiryukov committed Oct 29, 2024
1 parent 8d58139 commit 695476e
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 60 deletions.
129 changes: 75 additions & 54 deletions src/wp-includes/comment.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -773,59 +773,7 @@ function wp_allow_comment( $commentdata, $wp_error = false ) {
return new WP_Error( 'comment_flood', $comment_flood_message, 429 );
}

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 );
}

/**
Expand Down Expand Up @@ -1292,6 +1240,75 @@ function wp_check_comment_data_max_lengths( $comment_data ) {
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 );
}

/**
* Checks if a comment contains disallowed characters or words.
*
Expand Down Expand Up @@ -2279,11 +2296,15 @@ function wp_new_comment( $commentdata, $wp_error = false ) {

$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 );
}

if ( is_wp_error( $commentdata['comment_approved'] ) ) {
Expand Down
37 changes: 31 additions & 6 deletions tests/phpunit/tests/comment/wpHandleCommentSubmission.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -989,9 +989,8 @@ public function test_disallowed_keys_match_gives_approved_status_of_trash() {

$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.' );
}

/**
Expand All @@ -1009,8 +1008,34 @@ public function test_disallowed_keys_html_match_gives_approved_status_of_trash()

$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' => '[email protected]',
);

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.' );
}
}

0 comments on commit 695476e

Please sign in to comment.