Posted by NC on 08/05/07 04:50

On Aug 4, 8:01 pm, zach <wackzi...@gmail.com> wrote:
>
> I created a comment form which will inserts the comments into a database
> and displays them immediately. I want to make sure that its safe from
> users inserting unwanted data into the database or executing queries.
>
> Here's my php code, is this done right?
>
> $handle = mysql_connect($host,$user,$password)
> or die ('Sorry, lookslike an error occurred.');
> $sql = "INSERT INTO comments (id, comment, name, quotekey)
> VALUES (NULL, '$comment', '$name', '$key')";
> mysql_real_escape_string($sql);
> mysql_select_db($database);
> mysql_query($sql);
> mysql_close($handle);

Nope. You are escaping the whole query instead of just the inputs.
Additionally, you may or may not need to use
mysql_real_escape_string() depending on whether magic_quotes_gpc is
on. In an unrelated vein, there is no need to set an AUTO_INCREMENT
field to NULL in order to auto-increment it; you can simply omit it
from your SQL.

Consider something like this:

function escape_properly($arg) {
if (get_magic_quotes_gpc()) {
return mysql_real_escape_string(stripslashes($arg));
} else {
return mysql_real_escape_string($arg);
}
}

$handle = mysql_connect($host, $user, $password)
or die ('Sorry, lookslike an error occurred.');
$comment = escape_properly($comment);
$name = escape_properly($name);
$key = escape_properly($key);
$sql = 'INSERT INTO comments (comment, name, quotekey) ' .
"VALUES ('$comment', '$name', '$key')";
mysql_real_escape_string($sql);
mysql_select_db($database);
mysql_query($sql);
mysql_close($handle);

Cheers,
NC

[Back to original message]