Stopping users posting more than once

Question

Before posting my form I am checking the database to see if there are any previous posts from the user. If there are previous posts then the script will kick back a message saying you have already posted.

The problem is that what I am trying to achieve isn't working it all goes wrong after my else statement. It is also probable that there is an sql injection vulnerability too. Can you help??4

<?php

include '../login/dbc.php';
page_protect();

$customerid = $_SESSION['user_id'];

$checkid = "SELECT customerid FROM content WHERE customerid = $customerid";

if ($checkid = $customerid) {echo 'You cannot post any more entries, you have already created one';}

else

$sql="INSERT INTO content (customerid, weburl, title, description) VALUES
('$_POST[customerid]','$_POST[webaddress]','$_POST[pagetitle]','$_POST[pagedescription]')";

if (!mysql_query($sql))
  {
  die('Error: ' . mysql_error());
  }
echo "1 record added";

?>

Answer 1

You are missing curly brackets {}:

<?php

if ($checkid == $customerid) {echo 'You cannot post any more entries, you have already created one';}

else
{

$sql="INSERT INTO content (customerid, weburl, title, description) VALUES
('$_POST[customerid]','$_POST[webaddress]','$_POST[pagetitle]','$_POST[pagedescription]')";

if (!mysql_query($sql))
  {
  die('Error: ' . mysql_error());
  }
echo "1 record added";
}

?>

Answer 2

Re: sql injection - any time you trust data from your users you're vulnerable. Take your INSERT statement and sanitize it.

$sql = sprintf("INSERT INTO content (customerid, weburl, title, description) VALUES ('%d','%s','%s','%s')",
    $_POST['customerid'], //forced as digit
    mysql_real_escape_string($_POST['webaddress']),
    mysql_real_escape_string($_POST['pagetitle']),
    mysql_real_escape_string($_POST['pagedescription']) );

Also, you should use apostrophes in your array keys. In double quotes, that'd be:

echo "Post data webpage title is {$_POST['pagetitle']}";

Answer 3

To answer the second part of your question: yes, you're very vulnerable to SQL injection:

$sql="INSERT INTO content (customerid, ...) VALUES ('$_POST[customerid]', ...)";
                                                     ^

This article explains SQL Injection and how to avoid the vulnerability in PHP.

Answer 4

$_SESSION will clear when the browser is closed out. Therefore, I'd suggest using Cookies for a definite way.

I've updated your code as follows:

include '../login/dbc.php';
page_protect();

$customerid = $_COOKIE['user_id'];

$checkid = "SELECT customerid FROM content WHERE customerid = $customerid";

if ($checkid = $customerid) {echo 'You cannot post any more entries, you have already created one';}else{

$sql="INSERT INTO content (customerid, weburl, title, description) VALUES
('$_POST[customerid]','$_POST[webaddress]','$_POST[pagetitle]','$_POST[pagedescription]')";

if (!mysql_query($sql))
  die('Error: ' . mysql_error());
else
  echo "1 record added";
}

If you are worried about injection add this tidbit prior to your insert query:

foreach($_POST as $key=>$value){
  $_POST[$key] = addslashes($value);
}

Answer 5

In addition to the missing curly braces mentioned previously it looks like you're assigning in the if statement, which will cause the statement to always evaluate to true:

if ($checkid = $customerid) {echo 'You cannot post any more entries, you have already created one';}

Should be:

if ($checkid == $customerid) {echo 'You cannot post any more entries, you have already created one';}

Also, $checkid contains an SQL query string. I assume you intend to actually run the query and populate $checkid with something comparable to a $customerid before actually getting to the comparison.

Answer 6

In addition to the SQL injections (man, read a book/tutorial about that before you start!) and the missing braces after the else, you have two errors in there: First, you don't execute the $checkid query, secondly, you only have one = in the if (so you assign the value of $customerid to $checkid.

It is also probable that there is an sql injection vulnerability too.

Why "is possible"? Don't you see that yourself? Don't you write your code in a way that you avoid such issues in the first place?