Simple PHP function safe against SQL injection?

Question

I have a simple routine that deletes a row from an SQL database..

<?php
global $wpdb, user_ID;
$tmp_mid = $_GET['mid'];
if (!empty($tmp_mid))
{
    $id_check = $wpdb->get_var($wpdb->prepare("SELECT message_to_user_ID FROM " . $wpdb->base_prefix . "messages WHERE message_ID = %d", $tmp_mid));
    if ( $id_check == $user_ID )
    {
        $wpdb->query( $wpdb->prepare("DELETE FROM " . $wpdb->base_prefix . "messages WHERE message_ID = %d", $tmp_mid ));
    }
}
?>

I want to ensure that the row can only be deleted if the $user_ID matches the $tmp_mid from the row. All seems to work correctly but is this routine vulnerable to SQL injection?

Do I need to do anything to it to secure it?

This question appears to be off-topic because it is asking for a http://codereview.stackexchange.com/ — Quentin, Sep 28 '14 at 13:36
@Quentin I'm tempted to go along with the same vote to close, or http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 — Funk Forty Niner, Sep 28 '14 at 14:00
@Fred-ii- — It's a bit, IMO, wordpress specific to really play nicely with that question. — Quentin, Sep 28 '14 at 14:01
@Quentin So, go with the code review vote would be more appropriate then? — Funk Forty Niner, Sep 28 '14 at 14:02

Themis Beris · Accepted Answer · 2014-09-28T13:53:00.560

3

Your code is not vulnerable to SQL-Injection.

Edit: As DCoder pointed out, you are safe from sql-injection because this wp method runs finally the mysqli_real_escape_string() function where your input is sanitized properly ! Still you do not make use of prepared statements, but it's fine .

edited Sep 28 '14 at 13:53

answered Sep 28 '14 at 13:36

Themis Beris

980
1
11
25

2

The code in the question is already using prepared statements (not in a way that helps with SQL injection but you really need to elaborate). – Quentin Sep 28 '14 at 13:38
@Quentin i can't really find where is the use of prepared statements in the code given. I don't see use of the `bind_param` or other similar methods – Themis Beris Sep 28 '14 at 13:41
Prepared statements: http://note.io/1vpvdvr – Quentin Sep 28 '14 at 13:42
2

[WordPress developers created `wpdb::prepare` specifically to make it safe to use user input](http://codex.wordpress.org/Class_Reference/wpdb#Protect_Queries_Against_SQL_Injection_Attacks). Note that it still boils down to `mysql(i)_real_escape_string`, not real prepared statements. – DCoder Sep 28 '14 at 13:43
huh, looks like I was wrong. The code in the question seems to be using prepared statements in a secure way after all. Thanks to @DCoder for pointing to the WordPress manual. I hadn't recognised the significance of the `$wpdb` variable. – Quentin Sep 28 '14 at 13:44
Added wordpress tag to question. – mhawke Sep 28 '14 at 13:45
@DCoder I didn't know that `wpdb` is a wordpress thing. I have not worked with wp and as the tags of the question are `sql`,`sql-injection` and `php` i thought tha wpdb was just a custom db class. I will change my answer thanks! – Themis Beris Sep 28 '14 at 13:48

score 0 · Answer 2 · answered Sep 28 '14 at 13:39

0

Use mysql_real_escape_string.

$tmp_mid = mysql_real_escape_string($_GET['mid']);

If $tmp_mid can only be an integer, you could strip all non-numeric numbers from that variable.

You can't fully rely on that function all the time though for all situations.

answered Sep 28 '14 at 13:39

The code in the question doesn't appear to be using the (obsolete, deprecated, soon to be unavailable) `mysql_` interface, so that will likely fail due to not having an open connection using `mysql_`. – Quentin Sep 28 '14 at 13:41

Simple PHP function safe against SQL injection?

2 Answers2