Is $wpdb->prepare escaping to much? How to use it properly?

Question

Im pretty much confused by $wpdb->prepare. I want to use it to sanitize user input but it turned out that it destroys the query. I cannot imagine that this is how it's intended to work but I can't find out what I'm doing wrong. Please be so patient and point me to the solution. The following code is just a simple example for demonstration purposes just to make it clear what's going wrong and how. Later there will be user input feeding this query.

When I left out prepare and use sprintf instead this example is working :

global $wpdb; $mydb = new \wpdb( "My_Login" ,"My_PW" , "My_Custom_DB" ,"localhost"); //wpdb instance for my custom db $tablename = "books"; $sort_direction = "DESC"; $limit = 5; $sqlquery = sprintf( "SELECT * FROM %s ORDER BY datum %s LIMIT %d", $tablename, $sort_direction, $limit ); $res = $mydb->get_results( $sqlquery ); echo "<br>".$sqlquery."<br>"; var_dump($res);

The output of var_dump() is a proper array containing the expected result.

The output of $sqlquery is:

SELECT * FROM books ORDER BY datum DESC LIMIT 5

Which is proper SQL.

Now I will use prepare (this is the only changed line):

global $wpdb; $mydb = new \wpdb( "My_Login" ,"My_PW" , "My_Custom_DB" ,"localhost"); //wpdb instance for my custom db $tablename = "books"; $sort_direction = "DESC"; $limit = 5; $sqlquery = $mydb->prepare( "SELECT * FROM %s ORDER BY datum %s LIMIT %d", $tablename, $sort_direction, $limit ); $res = $mydb->get_results( $sqlquery ); echo "<br>".$sqlquery."<br>"; var_dump($res);

The output of var_dump() is NULL

The output of $sqlquery is:

SELECT * FROM 'books' ORDER BY datum 'DESC' LIMIT 5

Where obviously table name and DESC shouldn't be enclosed in quotation marks. IMO this is the reason why it fails. I double checked that this isn't related to instancing $wpdb. When I work with $wpdb the escaping result is the same.

What's going on or what did I wrong? Please help me.

You only need to use prepare on things that need to be sanitized, i.e. anything based on user input. Selecting the last five books is a static query, it's not ever going to change based on user input. Prepare won't break queries if it's used to sanitize the correct things. — Andrew Bartel
– Andrew Bartel, Commented Apr 4, 2014 at 20:28

Otto · Accepted Answer · 2014-04-05 22:25:06Z

What you did wrong here was to prepare those items in the first place.

You only run "data" variables through prepare(). You don't run table names, or sort directions, or limits through it. These are part of the SQL command itself, they are not data that refers to information which is stored in a column in the database.

Your SELECT query has no data inputs... therefore it does not have any inputs that can be prepare'd.

For the specific case of a SELECT, data is the stuff in the WHERE clause. column = %s and so on. That is a variable piece of data that can be potentially unsafe because it possibly comes from user input. So that data must be run through prepare to make it safe. But if you have a hardcoded "DESC" for the ordering, then there's no point in running it through prepare. It's "DESC". It's safe as is. Only data that you do not know what it is can be unsafe.

Edit: That said, if data does come from user input, like if the user can select the number of items to display, then that data must be sanitized. So limits can be run through prepare as they are integers and might come from a user selecting "5" or "10".

However, you wouldn't allow a user to directly input the table name, or sort order. They would pick a direction to display things, but you'd convert that into ASC or DESC. They would not pick "books" directly, but might select from a list which you interpret as "books". That sort of thing.

Thanks for clarifying that. Not it's clear to me. I found somewhere an example where someone used prepare for selecting the table too - obviously this one was erroneous and misled me to believe that I can use prepare for everything. But isn't there a wp equivalent to mysql_real_escape_string() ? — Hexodus
– Hexodus, Commented Apr 6, 2014 at 14:03
The esc_sql() function will escape data for you if you need to do that. It will eventually call mysql_real_escape_string(), or mysqli_real_escape_string() (in 3.9, if you have mysqli support). — Otto
– Otto, Commented Apr 6, 2014 at 19:20

Stack Exchange Network

Is $wpdb->prepare escaping to much? How to use it properly?

1 Answer 1

Hot Network Questions

Is $wpdb->prepare escaping to much? How to use it properly?

1 Answer 1

Related

Hot Network Questions