How do i secure this PHP script?

Question

I'm worried about sql injection, so how do i prevent it? I'm using this script but have had several people tell me its very insecure, if anyone can help by telling me how it would be great :).

source code:

if(isset($_POST['lastmsg']))
{
$lastmsg=$_POST['lastmsg'];
$result=mysql_query("SELECT * FROM updates WHERE item_id<'$lastmsg' ORDER BY item_id DESC LIMIT 16");
$count=mysql_num_rows($result);
while($row=mysql_fetch_array($result))
{
$msg_id=$row['item_id'];
$message=$row['item_content'];

Is `$lastmsg` a numeric value? – Pekka Mar 31 '11 at 22:40 — Pekka, Mar 31 '11 at 22:40

score 6 · Accepted Answer · answered Mar 31 '11 at 22:40

6

Never, ever, put information from the user ($_POST or $_GET) directly into a query. If they are numbers, always convert them to integers first with (int)$var or intval($var); if they are strings, always escape them with mysql_real_escape_string().

Read http://us2.php.net/mysql_real_escape_string and use it.

answered Mar 31 '11 at 22:40

Andrew

5,095
6
42
48

**Note:** Make sure you use `mysql_real_escape_string` NOT `mysql_escape_string` – Christian Mar 31 '11 at 22:41

score 2 · Answer 2 · answered Mar 31 '11 at 22:41

2

$lastmsg = intval($_POST['lastmsg']);

answered Mar 31 '11 at 22:41

Dejan Marjanović

19,244
7
52
66

This is the correct answer in this case. A check for a failed `intval` would be nice though, otherwise it will default to `0` which can have unintended consequences – Pekka Mar 31 '11 at 22:41
Feel free to edit :) Thanks for suggestions. Yes, he should check if it is > 0. – Dejan Marjanović Mar 31 '11 at 22:43

score 1 · Answer 3 · answered Mar 31 '11 at 23:09

1

The best solution is migrating to mysqli_ or PDO and using prepared statements for your queries.

answered Mar 31 '11 at 23:09

Bv202

3,924
13
46
80

Ibu · Answer 4 · 2011-03-31T23:01:24.060

0

You can simply add this :

$lastmsg=addslashes($_POST['lastmsg']);

for a larger scale you can use a more robust solution here is the function you can use

function escape_value($value) {
    $magic_quotes_active = get_magic_quotes_gpc();
    $new_php = function_exists("mysql_real_escape_string");
    if ($new_php){
        if ($magic_quotes_active){
            $value = stripslashes($value);
            }
        $value = mysql_real_escape_string($value);
    } else {
        if(!$magic_quotes_active) { $value = addslashes($value); }
    }
    return trim($value);
}

mysql_real_escape_string is not always available so here is a work around

edited Mar 31 '11 at 23:01

answered Mar 31 '11 at 22:42

Ibu

42,752
13
76
103

3

`addslashes` is not the right choice. You need to use `mysql_real_escape_string()` to be protected from injection. See http://stackoverflow.com/questions/3473047/mysql-real-escape-string-vs-addslashes – Pekka Mar 31 '11 at 22:43
1

Quoting the PHP documentation: "It's highly recommended to use DBMS specific escape function (e.g. mysqli_real_escape_string() for MySQL)" – Andrew Mar 31 '11 at 22:43

Joe Phillips · Answer 5 · 2011-03-31T22:55:30.823

Make sure the magic_quotes_gpc directive is disabled in your PHP configuration (PHP.ini).

If lastmsg is a string and you are using mysql, do this:

$lastmsg = mysql_real_escape_string($_POST['lastmsg']);

If lastmsg is a string and your DBMS does not have a native escape function, do this:

$lastmsg = addslashes($_POST['lastmsg']);

If lastmsg should be a number, check for that instead and don't do any escaping. You can use function is_numeric() and then cast the numeric string to an integer using intval().

How do i secure this PHP script?

5 Answers5