0

I'm trying to optimize SQL from sql injects using $wpdb->prepare();

This query doesn't return anything. Can anyone see why or have a more elegant alternative. Also is it necessary to escape the search terms in the MATCH AGAINST statement.

$search_terms = "example search" // input post
// remove single quotes from search terms
$search_terms = str_replace("'","",$search_terms);

$sql = "SELECT
    post.ID,
    post.post_author,
    post.post_date,
    post.post_title,
    LEFT(post.post_content, 240) As post_content,
    post.post_name,
    post.post_type,
    post.comment_count,
    post.comment_status,
    MATCH (post_search.post_content,post_search.post_title) AGAINST (%s IN BOOLEAN MODE) AS score
FROM wp_posts post
LEFT JOIN wp_term_relationships term_rel
    ON post.ID = term_rel.object_id
LEFT JOIN wp_term_taxonomy term_tax
    ON term_rel.term_taxonomy_id = term_tax.term_taxonomy_id
LEFT JOIN wp_terms terms
    ON term_tax.term_id = terms.term_id
LEFT JOIN wp_posts_fulltext_search post_search 
    ON post.ID=post_search.post_id
WHERE MATCH(post_search.post_content,post_search.post_title) AGAINST (%s IN BOOLEAN MODE) && 
    post.post_status = 'publish' && 
    post.post_type = 'post' && 
    (term_tax.description != '' && 
    term_tax.description NOT LIKE '%sample%') 
GROUP BY post.ID
ORDER BY score DESC
LIMIT 0,20";

$results = $wpdb->get_results( $wpdb->prepare( $sql, $search_terms ) );

The "As score" is used to return more relevant results to the top. Just to reiterate I'm more concerned about the SQL inject, MATCH AGAINST and wordpress prepare function.

Johan
  • 74,508
  • 24
  • 191
  • 319
madphp
  • 1,716
  • 5
  • 31
  • 72

2 Answers2

2

Don't ever use this code:

// remove single quotes from search terms 
$search_terms = str_replace("'","",$search_terms);

It's broken, because it does not take MySQL's encoding into account.

If you want to escape, use this instead:

$search_terms = mysql_real_escape_string($search_terms);

But beware!, this will only escape values, not table names and not field names.
If you want manipulate the field or table names in the query the rules are:

  1. Don't do it you are asking for SQL-injection trouble.
  2. Don't do it with PDO either, you're still running into SQL-injection problems.
  3. Read the answer by Pekka to my question: How to prevent SQL injection with dynamic tablenames?

However
No escaping is needed, because you're using a prepared statement.

Community
  • 1
  • 1
Johan
  • 74,508
  • 24
  • 191
  • 319
  • thanks for your advice. I've changed that. It still doesnt bring back results when I try to use MATCH AGAINST (%s IN BOOLEAN MODE) and $wpdb->prepare though. – madphp Apr 28 '11 at 18:05
  • are you saying, if i use mysql_real_escape_string() it isnt nessecary to use prepared statement. – madphp Apr 28 '11 at 18:07
  • @madphp, No the other way round you don't need mysql_real_escape_string() when using PDO's. PDO's are more secure than using mysql_real_escape_string(), but not 100% secure. – Johan Apr 28 '11 at 20:13
  • @madphp, see: http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain for more info on SQL-injection. – Johan Apr 28 '11 at 20:17
0

Copy pasted from http://codex.wordpress.org/Function_Reference/wpdb_Class

Protect Queries Against SQL Injection Attacks

For a more complete overview of SQL escaping in WordPress, see database Data Validation. That Data Validation article is a must-read for all WordPress code contributors and plugin authors.

Briefly, though, all data in SQL queries must be SQL-escaped before the SQL query is executed to prevent against SQL injection attacks. This can be conveniently done with the prepare method, which supports both a sprintf()-like and vsprintf()-like syntax.

<?php 
  $sql = $wpdb->prepare('query' [, value_parameter, value_parameter ... ] ); 
?>

query
(string) The SQL query you wish to execute, with %s and %d placeholders. Any other % characters may cause parsing errors unless they are escaped. All % characters inside SQL string literals, including LIKE wildcards, must be double-% escaped as %%. (<<-- !!!)

_value_parameter_
(int|string|array) The value to substitute into the placeholder. Many values may be passed by simply passing more arguments in a sprintf()-like fashion. Alternatively the second argument can be an array containing the values as in PHP's vsprintf() function. Care must be taken not to allow direct user input to this parameter, which would enable array manipulation of any query with multiple placeholders. Values must not already be SQL-escaped.

Examples

Add Meta key => value pair "Harriet's Adages" => "WordPress' database interface is like Sunday Morning: Easy." to Post 10.

$metakey = "Harriet's Adages";
$metavalue = "WordPress' database interface is like Sunday Morning: Easy.";

$wpdb->query( $wpdb->prepare( "
    INSERT INTO $wpdb->postmeta
    ( post_id, meta_key, meta_value )
    VALUES ( %d, %s, %s )", 
        10, $metakey, $metavalue ) );

Performed in WordPress by add_meta().

The same query using vsprintf()-like syntax.

$metakey = "Harriet's Adages";
$metavalue = "WordPress' database interface is like Sunday Morning: Easy.";

$wpdb->query( $wpdb->prepare( "
    INSERT INTO $wpdb->postmeta
    ( post_id, meta_key, meta_value )
    VALUES ( %d, %s, %s )", 
        array(10, $metakey, $metavalue) ) );

(Part of) Your problem

You did not double escape your '%' in the like statement.
Change: 

term_tax.description NOT LIKE '%sample%')

to

term_tax.description NOT LIKE '%%sample%%')
Johan
  • 74,508
  • 24
  • 191
  • 319