Unsafe SQL calls

  • Vishal

    (@vishaligex)


    2 months ago



    I create one custom plugin and submit for review

    bellow is issues found by wordpress team

    includes/databases/class-stepup-user-crud.php:313 $sql_orders = $wpdb->prepare( " SELECT p.* FROM {$db->tb_posts} p INNER JOIN {$db->tb_postmeta} pm ON p.ID = pm.post_id AND meta_key = %s AND (meta_value = %d OR meta_value like '%s') ", '_user_id', $user_id, $user_id_str ); includes/databases/class-stepup-user-crud.php:338 $sql = $sql_orders /* . ' UNION ' . $sql_guest_orders */ . $sql_rest; includes/databases/class-stepup-user-crud.php:341 $order_posts = $db->wpdb->get_results($sql);

    # There is a call to a wpdb::prepare() function, that's correct. # You cannot add variables like "$db->tb_posts" directly to the SQL query. # Using wpdb::prepare($query, $args) you will need to include placeholders for each variable within the query and include the variables in the second parameter. # The SQL query needs to be included in a wpdb::prepare($query, $args) function.

    I do not understand problem in these like “You cannot add variables like “$db->tb_posts” directly to the SQL query.” I have one global variable define for $db->tb_posts.

    so please help me

Viewing 2 replies - 1 through 2 (of 2 total)
  • Moderator bcworkz

    (@bcworkz)

    2 months ago

    Globals are still variables. You shouldn’t have any variables in a SQL query passed to wpdb::prepare(), of any sort. Are there safe ways to do so? Perhaps, but it’s simpler to not do so when the alternative is not at all burdensome.

    Replace the variable in your SQL with the appropriate placeholder, then pass the actual variable as one of the subsequent parameters.

    iamcallen

    (@iamcallen)

    2 months ago

    The review team is being strict here because of how wpdb::prepare() works. It only sanitizes values that go into the query (things you’d replace with %s, %d, etc.). It does not escape or protect table names or column names. That’s why they don’t allow you to drop variables like $db->tb_posts directly into the SQL string.

    For core tables you don’t need your own $db->tb_posts anyway, WordPress already exposes them through $wpdb, e.g. $wpdb->posts and $wpdb->postmeta. Those are safe to use in prepared queries, since WP itself defines them. Example:

    global $wpdb; $sql_orders = $wpdb->prepare( "SELECT p.* FROM {$wpdb->posts} p INNER JOIN {$wpdb->postmeta} pm ON p.ID = pm.post_id WHERE pm.meta_key = %s AND (pm.meta_value = %d OR pm.meta_value LIKE %s)", '_user_id', $user_id, $user_id_str ); $order_posts = $wpdb->get_results($sql_orders);

    If you’re working with custom tables instead of core WP tables, the best practice is to define the table name as a constant during plugin activation (e.g. define( 'MYPLUGIN_USERS_TABLE', $wpdb->prefix . 'myplugin_users' );) and then reference that constant in your queries. That way, reviewers see it’s not user input, and your code passes review more smoothly.

    So in short: keep variables out of the query string, let prepare() handle values only, and use $wpdb->tablename or constants for table names.

Viewing 2 replies - 1 through 2 (of 2 total)

You must be logged in to reply to this topic.