Use sql query in str_replace

Question

I have a code for getting post title like this:

$content = str_replace('%title', $post->post_title, $content );

I'd like to use my own title from database.
I wrote this code:

        global $wpdb;
        $mycontent = $wpdb->get_var(
            'SELECT `meta_value` FROM `wp_postmeta` WHERE `post_id` = '.$post->ID.' AND `meta_key` = \'my_seo_title\';'
        );
        $content = str_replace('%my_seo_title', $mycontent , $content);

Does it make a security problem?

Possible duplicate of [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — Norbert, Oct 27 '18 at 18:24
What you are trying to do is really dangerous: Very easy to hack. See the "duplicate" I linked for how to SQL without the security risk — Norbert, Oct 27 '18 at 18:25
which mysql driver you are using PDO .. mysqli ?.. you could do what you need using a bining param — ScaisEdge, Oct 27 '18 at 18:33
@norbert-van-nobelen I've used your link to edit my question. **Does my coding make a security trouble?** (`$wpdb` is for wordpress sql connection) — sarah miller, Oct 27 '18 at 20:42
So, you're trying to replace `%my_seo_title` with the value of a custom field called `my_seo_title`? — cabrerahector, Oct 27 '18 at 21:29
Yes, concatenating user accessible data with SQL can lead to the mentioned SQL injection. Someone could add a `"; SHOW TABLES;` for example by hacking your `POST` — Norbert, Oct 28 '18 at 00:37

score 1 · Accepted Answer · answered Oct 27 '18 at 21:55

Does it have security issue?

In the unlikely event that your $post object gets replaced with something else (and at that point I'd consider the website's security as already compromised), the attacker could replace the value returned by $post->ID with a malicious query string (a.k.a. SQL Injection).

To avoid that, as everyone else already pointed out, you should escape your query using the prepare() method from the $wpdb object:

$mycontent = $wpdb->get_var(
    $wpdb->prepare(
        "SELECT `meta_value` FROM `wp_postmeta` WHERE `post_id` = %d AND `meta_key` = %s;",
        array( $post->ID, 'my_seo_title' )
    )
);

Out of curiosity, why are you manually retrieving the meta value from the database when we already have the get_post_meta() function (which does the whole security check automagically for you)? I mean, you could replace your code with:

$mycontent = get_post_meta( $post->ID, 'my_seo_title', true );

... and forget about writing queries by hand and/or making them secure (when not necessary).

Thanks a million @cabrerahector . I was in a wrong way (thinking of retrieving is necessary when the data is in another table) ; Your code code worked very well . — sarah miller, Oct 28 '18 at 08:24

Use sql query in str_replace

1 Answers1