1

I have a page on my site with numerous elements that can be liked/disliked through a simple admin-ajax call.

In the associated PHP code, it looks like this:

// Fetch the existing meta from this element. The array of users who already voted, and the current score.
global $wpdb;
$row = $wpdb->get_row( "SELECT `rating_users`, `rating_score` FROM `table` WHERE `file_id` = $file_id" );

$voted_users   = $row->rating_users? unserialize( $row->rating_users) : array();
$current_score = $row->rating_score ?: 0;

// Now verify this current user has not already voted, check the array.
foreach ($voted_users as $id => $ip)
{
    // check if current user ID / IP has voted, exit if true...
}

// Good to go, add this user to the list of users who have voted now.
$voted_users[] = array(
    $user_id => $user_ip
);
$current_score++;

$wpdb->update(
    'table',
    array(
        'rating_users' => serialize( $voted_users ),
        'rating_score ' => $current_score
    ),
    array(
        'file_id' => $file_id
    ),
    array( '%s', '%d' ),
    array( '%d' )
);

So I feel like in between the time I (retrieve the current list of users who have voted / score...verify if this is a valid vote...and then adjust/update the values back into the database) another user could have initiated the same process - and thus are dealing with "wrong" data. Data would be lost as one process would override the other's upon the wpdb->update call.

I haven't verified this, as emulating a race condition is difficult, but seems valid - what is a better method of performing the above that wouldn't result in this issue?

1
  • By using serialize and unserialize you're exposed to Object deserialisation attacks. Just store the user IDs as a comma separated list, or JSON
    – Tom J Nowell
    Mar 31, 2020 at 23:30

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Browse other questions tagged or ask your own question.