PHP Ban Visitor & Insert into mySQL DB.

Question

I have the below code that i am wanting to into certain files so that when someone visits this "certain" file they get banned if they are not allready. but for some reason it is not adding new visitors into the database, if i add a user manually it works fine and echo's Banned! but otherwise it just echo's the $sql query but does not actually do it.

<?php
$host=""; // Host name
$username=""; // Mysql username
$password=""; // Mysql password
$db_name="banlist"; // Database name
$tbl_name="list"; // Table name

// Connect to server and select databse.
mysql_connect("$host", "$username", "$password")or die("cannot connect");
mysql_select_db("$db_name")or die("cannot select DB");

// Define $myusername and $mypassword
$ip = isset($_SERVER['HTTP_X_FORWARDED_FOR']) ? $_SERVER['HTTP_X_FORWARDED_FOR'] : $_SERVER['REMOTE_ADDR'];

$sql="SELECT * FROM $tbl_name WHERE ip='$ip'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);
// If result matched $myusername and $mypassword, table row must be 1 row

if ($count==0){
$sql="INSERT INTO $tbl_name (`id` ,`ip`) VALUES (NULL , $ip)";
mysql_query($sql);
echo $sql;
//header("location:index.html");
} else {
// Register $myusername, $mypassword and redirect to file "login_success.php"
//header("location:index.html");
echo "banned!";
exit();
}
?>

Answer 1

just few remarks

$sql="SELECT * FROM $tbl_name WHERE ip='$ip'";
$result=mysql_query($sql);

wouldn't be better to do a

$sql="SELECT count(*) FROM $tbl_name WHERE ip='$ip'";
$result=mysql_query($sql);

since you don't use that data.

$sql="INSERT INTO $tbl_name (`id` ,`ip`) VALUES (NULL , '$ip')";
mysql_query($sql);

if your id is an auto increment you don't have to include it

$sql="INSERT INTO $tbl_name (`ip`) VALUES ('$ip')";
mysql_query($sql);

You should quote $ip since it's probably a varchar in your table.

Since an ip address should be a sort of unique identifier you have better to use the IP as primary key. last point checking for results of mysql_query would be a good pratice, like there

$sql="INSERT INTO $tbl_name (`ip`) VALUES ($ip)";
$ret = mysql_query($sql);
if (!$ret) {
    die('Invalid query: ' . mysql_error());
}

I think it would give you valuable information about what is happening. in that case it would probably say you have an error near the IP address (because of the missing quotes).

Answer 2

Have you double-checked that your MySQL account has the INSERT privilege?

You'll also find that things go more smoothly if you always check the return value of mysql_query(). While you're developing, you could change these lines (from the end of your snippet):

mysql_query($sql);
echo $sql;

... to this:

$result = mysql_query($sql);

if($result === FALSE) {
    echo 'INSERT failed with this error: '.mysql_error();
} else {
    echo 'INSERT succeeded';
}

Also if you're not yet familiar with SQL injection, you'll want to become familiar with it. Your code is currently vulnerable to this kind of attack, because it doesn't filter input (the HTTP headers where you're looking for an IP address) and it doesn't escape output (the variable portion of your dynamically-constructed SQL queries).